This is the mail archive of the cygwin-patches mailing list for the Cygwin project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
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
Attachment:
signature.asc
Description: PGP signature
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |