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

On Wed, 20 Feb 2013 15:32:10 +0100
Matteo Casalin <matteo.casalin@gmx.com> wrote:

> 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};

As an additional thought, this could then be :

      char pReceiveBuffer[sc_nCSASeqLength + 1];

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