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

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@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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/libreoffice