Re: [PATCH] cygcheck bug when listing services
On Tue, 6 Dec 2005, Christopher Faylor wrote: > On Tue, Dec 06, 2005 at 08:41:24PM -0500, Igor Pechtchanski wrote: > >On Tue, 6 Dec 2005, Yitzchak Scott-Thoennes wrote: > >[snip] > >> That needs a comment in the code. > > > >Fair enough: > >[snip] > >+ /* Add two nulls to avoid confusing strtok() when the trailing > >separator > >+ is missing */ > > How about a testcase which shows that the MSVCRT strtok needs two > trailing NUL bytes to work around problems when there is no trailing > separator? I would find that much more interesting than a comment which > simply asserts that behavior. Fair enough. The double-null issue was a red herring, apparently -- a simple recompile fixed the problem. In fact, the cygcheck from the latest snapshot doesn't exhibit this behavior. Patch withdrawn. Igor -- http://cs.nyu.edu/~pechtcha/ |\ _,,,---,,_[EMAIL PROTECTED] ZZZzz /,`.-'`'-. ;-;;,_[EMAIL PROTECTED] |,4- ) )-,_. ,\ ( `'-' Igor Pechtchanski, Ph.D. '---''(_/--' `-'\_) fL a.k.a JaguaR-R-R-r-r-r-.-.-. Meow! If there's any real truth it's that the entire multidimensional infinity of the Universe is almost certainly being run by a bunch of maniacs. /DA -- Unsubscribe info: http://cygwin.com/ml/#unsubscribe-simple Problem reports: http://cygwin.com/problems.html Documentation: http://cygwin.com/docs.html FAQ: http://cygwin.com/faq/
Re: [PATCH] cygcheck bug when listing services (Was Re: Parallel writes to a single FIFO do not queue, and deadlock cygwin)
On Tue, Dec 06, 2005 at 08:41:24PM -0500, Igor Pechtchanski wrote:
>On Tue, 6 Dec 2005, Yitzchak Scott-Thoennes wrote:
>
>> On Mon, Dec 05, 2005 at 09:40:01AM -0500, Igor Pechtchanski wrote:
>> > Running cygcheck under strace shows that after listing all the available
>> > services, it invokes "cygrunsrv --query grunsrv.exe --list", which results
>> > in the above message. I think this may be because the output of
>> > "cygrunsrv --list" doesn't contain a trailing '\n', and so strtok gets
>> > confused. The following patch (against a slightly older cygcheck) fixes
>> > it for me, but I haven't had the time to test it extensively:
>> > [snip]
>> > - buf[nchars] = 0;
>> > + buf[nchars] = buf[nchars+1] = 0;
>>
>> That needs a comment in the code.
>
>Fair enough:
>
>Index: cygcheck.cc
>===
>RCS file: /cvs/src/src/winsup/utils/cygcheck.cc,v
>retrieving revision 1.77
>diff -u -p -r1.77 cygcheck.cc
>--- cygcheck.cc 17 Aug 2005 00:52:43 - 1.77
>+++ cygcheck.cc 7 Dec 2005 01:38:07 -
>@@ -950,8 +950,10 @@ dump_sysinfo_services ()
> else
> {
> /* read the output of --list, and then run --query for each service */
>- size_t nchars = fread ((void *) buf, 1, sizeof (buf) - 1, f);
>- buf[nchars] = 0;
>+ size_t nchars = fread ((void *) buf, 1, sizeof (buf) - 2, f);
>+ /* Add two nulls to avoid confusing strtok() when the trailing separator
>+ is missing */
>+ buf[nchars] = buf[nchars+1] = 0;
> pclose (f);
>
> if (nchars > 0)
>
>I'll even add a ChangeLog of sorts :-)
>
>2005-12-06 Igor Pechtchanski <[EMAIL PROTECTED]>
>
> * cygcheck.cc (dump_sysinfo_services): Add an extra NUL to mollify
> strtok() when the trailing newline is missing.
How about a testcase which shows that the MSVCRT strtok needs two
trailing NUL bytes to work around problems when there is no trailing
separator? I would find that much more interesting than a comment which
simply asserts that behavior.
cgf
--
Unsubscribe info: http://cygwin.com/ml/#unsubscribe-simple
Problem reports: http://cygwin.com/problems.html
Documentation: http://cygwin.com/docs.html
FAQ: http://cygwin.com/faq/
Re: [PATCH] cygcheck bug when listing services (Was Re: Parallel writes to a single FIFO do not queue, and deadlock cygwin)
On Tue, 6 Dec 2005, Yitzchak Scott-Thoennes wrote:
> On Mon, Dec 05, 2005 at 09:40:01AM -0500, Igor Pechtchanski wrote:
> > Running cygcheck under strace shows that after listing all the available
> > services, it invokes "cygrunsrv --query grunsrv.exe --list", which results
> > in the above message. I think this may be because the output of
> > "cygrunsrv --list" doesn't contain a trailing '\n', and so strtok gets
> > confused. The following patch (against a slightly older cygcheck) fixes
> > it for me, but I haven't had the time to test it extensively:
> > [snip]
> > - buf[nchars] = 0;
> > + buf[nchars] = buf[nchars+1] = 0;
>
> That needs a comment in the code.
Fair enough:
Index: cygcheck.cc
===
RCS file: /cvs/src/src/winsup/utils/cygcheck.cc,v
retrieving revision 1.77
diff -u -p -r1.77 cygcheck.cc
--- cygcheck.cc 17 Aug 2005 00:52:43 - 1.77
+++ cygcheck.cc 7 Dec 2005 01:38:07 -
@@ -950,8 +950,10 @@ dump_sysinfo_services ()
else
{
/* read the output of --list, and then run --query for each service */
- size_t nchars = fread ((void *) buf, 1, sizeof (buf) - 1, f);
- buf[nchars] = 0;
+ size_t nchars = fread ((void *) buf, 1, sizeof (buf) - 2, f);
+ /* Add two nulls to avoid confusing strtok() when the trailing separator
+ is missing */
+ buf[nchars] = buf[nchars+1] = 0;
pclose (f);
if (nchars > 0)
I'll even add a ChangeLog of sorts :-)
2005-12-06 Igor Pechtchanski <[EMAIL PROTECTED]>
* cygcheck.cc (dump_sysinfo_services): Add an extra NUL to mollify
strtok() when the trailing newline is missing.
HTH,
Igor
--
http://cs.nyu.edu/~pechtcha/
|\ _,,,---,,_[EMAIL PROTECTED]
ZZZzz /,`.-'`'-. ;-;;,_[EMAIL PROTECTED]
|,4- ) )-,_. ,\ ( `'-' Igor Pechtchanski, Ph.D.
'---''(_/--' `-'\_) fL a.k.a JaguaR-R-R-r-r-r-.-.-. Meow!
If there's any real truth it's that the entire multidimensional infinity
of the Universe is almost certainly being run by a bunch of maniacs. /DA
--
Unsubscribe info: http://cygwin.com/ml/#unsubscribe-simple
Problem reports: http://cygwin.com/problems.html
Documentation: http://cygwin.com/docs.html
FAQ: http://cygwin.com/faq/
