On Oct 10 14:26, Michael Haubenwallner wrote:
> On 10/10/2017 01:48 PM, Corinna Vinschen wrote:
> > Hi Michael,
> >
> > On Oct 9 18:58, Michael Haubenwallner wrote:
> >> When fork fails, we can use "%s" now with system_sprintf for the errmsg
> >> rather than a (potentially too small) buffer for the format string.
> >
> > How could buf be too small?
>
> See below.
>
> Actually I've found this by searching for suspect char array definitions
> while hunting the "uninitialized variable for RtlLookupFunctionEntry" bug.
>
> >> * fork.cc (fork): Use "%s" with system_printf now.
> >> ---
> >> winsup/cygwin/fork.cc | 9 ++-------
> >> 1 file changed, 2 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/winsup/cygwin/fork.cc b/winsup/cygwin/fork.cc
> >> index 73a72f530..bcbef12d8 100644
> >> --- a/winsup/cygwin/fork.cc
> >> +++ b/winsup/cygwin/fork.cc
> >> @@ -618,13 +618,8 @@ fork ()
> >> if (!grouped.errmsg)
> >> syscall_printf ("fork failed - child pid %d, errno %d",
> >> grouped.child_pid, grouped.this_errno);
> >> else
> >> - {
> >> - char buf[strlen (grouped.errmsg) + sizeof ("child %d - , errno
> >> 4294967295 ")];
>
> Usually child_pid is longer than the 2 characters counted by "%d", but
> errno usually is shorther than the 10 characters counted by "4294967295",
> and there is another 2 reserved characters counted by trailing " ".
>
> In practice the buffer unlikely will be too small, so this is merely
> cosmetics.
But buf is just the format string. It won't get manipulated by
system_printf. Which means the 4294967295 is nonsense, too, a %d
would have been sufficient.
> >> - strcpy (buf, "child %d - ");
> >> - strcat (buf, grouped.errmsg);
> >> - strcat (buf, ", errno %d");
> >> - system_printf (buf, grouped.child_pid, grouped.this_errno);
> >> - }
> >> + system_printf ("child %d - %s, errno %d", grouped.child_pid,
> >> + grouped.errmsg, grouped.this_errno);
> >>
> >> set_errno (grouped.this_errno);
> >> }
> >> --
> >> 2.14.2
> >
> > I guess this also means we can drop the if/else, kind of like
> >
> > system_printf ("child %d %s%s, errno %d",
> > grouped.child_pid,
> > grouped.errmsg ? "- " : "",
> > grouped.errmsg ?: "",
> > grouped.this_errno);
> >
> > What do you think?
>
> Nothing I really take care of - yet suggesting:
>
> system_printf ("fork failed - child %d%s%s, errno %d",
> grouped.child_pid,
> grouped.errmsg ? " - " : "",
> grouped.errmsg ?: "",
> grouped.this_errno);
Yep.
> But wait, what's the difference between syscall_printf and system_printf?
Prefixing with timestamps and stuff.
Corinna
--
Corinna Vinschen Please, send mails regarding Cygwin to
Cygwin Maintainer cygwin AT cygwin DOT com
Red Hat
signature.asc
Description: PGP signature
