[Libreoffice-commits] core.git: desktop/source

[Matteo Casalin]

Hi all,
    I may be completely wrong and I really don't know this code and what it is aimet to, but I'm worried that

    if (nBytes > 0)
    {
        pReceiveBuffer[nBytes-1] = 0;
    }

overwrites a received character (that could be '\0' or not).

If I understand correctly, the target is to have the sequence of characters being '\0' terminated whether that '\0' whas in the stream or not, and then I would say that the whole routine should be something like:

    char pReceiveBuffer[sc_nCSASeqLength + 1] = {0};
    int nResult = 0;
    int nBytes = 0;
    // read byte per byte
    while ((nResult=aStreamPipe.recv( pReceiveBuffer+nBytes, sc_nCSASeqLength-nBytes))>0)
    {
        nBytes += nResult;
        if (pReceiveBuffer[nBytes-1]=='\0')
        {
            break;
        }
    }
    // unconditionally end stream with '\0'
    pReceiveBuffer[nBytes] = '\0';

In case a '\0' is already in the sequence and the additional '\0' is undesired, nBytes can be adjusted in the "if (pReceiveBuffer[nBytes-1]=='\0')" so to overwrite the existing one.
Also, the "read byte per byte" comment looks wrong to me.

If I misunderstood the point then apologies for the noise.
Cheers

Matteo



On Wed, 20 Feb 2013 05:54:50 -0800 (PST)
julien2412 <serval2412 at yahoo.fr> wrote:

> Lubos Lunak wrote
> > On Tuesday 19 of February 2013, Julien Nabet wrote:
> >> diff --git a/desktop/source/app/officeipcthread.cxx
> >> b/desktop/source/app/officeipcthread.cxx index 8db7946..445ccb4 100644
> >> --- a/desktop/source/app/officeipcthread.cxx
> >> +++ b/desktop/source/app/officeipcthread.cxx
> >> @@ -497,23 +497,17 @@ OfficeIPCThread::Status
> >> OfficeIPCThread::EnableOfficeIPCThread() else if( pThread->maPipe.create(
> >> aPipeIdent.getStr(), osl_Pipe_OPEN, rSecurity )) // Creation not
> >> successfull, now we try to connect {
> >>              osl::StreamPipe aStreamPipe(pThread->maPipe.getHandle());
> >> -            char pReceiveBuffer[sc_nCSASeqLength + 1];
> >> +            char pReceiveBuffer[sc_nCSASeqLength + 1] = {0};
> >>              int nResult = 0;
> >>              int nBytes = 0;
> >>              int nBufSz = sc_nCSASeqLength + 1;
> >>              // read byte per byte
> >> -            pReceiveBuffer[0] = 0;
> >>              while ((nResult=aStreamPipe.recv( pReceiveBuffer+nBytes,
> >> nBufSz-nBytes))>0) { nBytes += nResult;
> >>                  if (pReceiveBuffer[nBytes-1]=='\0') {
> >>                      break;
> >>                  }
> >>              }
> >> -            /* make sure the buffer is \0 terminated */
> >> -            if (nBytes > 0)
> >> -            {
> >> -                pReceiveBuffer[nBytes-1] = 0;
> >> -            }
> >  Did you really mean to remove this part?
> 
> Hi Lubos,
> 
> Yes I meant it, why? Is it wrong?
> if "pReceiveBuffer" is initialized with 0 for the (sc_nCSASeqLength + 1)
> elements thanks to = {0} initialization, what obvious thing did I miss? Why
> "pReceiveBuffer[nBytes-1] = 0;" would need to stay?
> 
> Of course, if my commit is wrong, I'll revert it after my day time job
> 
> Julien
> 
> 
> 
> --
> View this message in context: http://nabble.documentfoundation.org/Re-Libreoffice-commits-core-git-desktop-source-tp4038892p4038893.html
> Sent from the Dev mailing list archive at Nabble.com.
> _______________________________________________
> LibreOffice mailing list
> LibreOffice at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/libreoffice