This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v4][BZ #15346] Allow leading and trailing spaces in getdate
- From: Roland McGrath <roland at hack dot frob dot com>
- To: Siddhesh Poyarekar <siddhesh at redhat dot com>
- Cc: OndÅej BÃlka <neleai at seznam dot cz>, GNU C Library <libc-alpha at sourceware dot org>
- Date: Tue, 9 Apr 2013 16:34:42 -0700 (PDT)
- Subject: Re: [PATCH v4][BZ #15346] Allow leading and trailing spaces in getdate
- References: <20130408125233 dot GS32556 at spoyarek dot pnq dot redhat dot com> <20130408132913 dot GA9017 at domone dot kolej dot mff dot cuni dot cz> <20130408141120 dot GA15689 at spoyarek dot pnq dot redhat dot com> <20130408201256 dot E3C762C09F at topped-with-meat dot com> <20130409043315 dot GC15689 at spoyarek dot pnq dot redhat dot com> <20130409163919 dot C10772C07E at topped-with-meat dot com> <20130409171731 dot GR15689 at spoyarek dot pnq dot redhat dot com>
> + size_t inlen, oldlen;
> + bool free_instr = false;
The general preference is not to put a local's definition any earlier than
it needs to be, i.e. put inlen/oldlen where they are initialized below and
put free_instr immediately above the if block that might set it.
> + char *instr;
> +
> + if (inlen == oldlen)
> + instr = (char *) string;
It's nice to avoid de-consting casts. e.g. replace INSTR with
COPIED_STRING, set to NULL in the default case. Then do:
char *copied_string = NULL;
if (inlen < oldlen)
{
bool using_malloc = false;
if (__libc_use_alloca (inlen + 1))
copied_string = alloca (inlen + 1);
else
{
copied_string = malloc (inlen + 1);
if (copied_string == NULL)
{
fclose (fp);
return 6;
}
using_malloc = true;
}
memcpy (copied_string, string, inlen);
copied_string[inlen] = '\0';
string = copied_string;
if (!using_malloc)
copied_string = NULL;
}
and:
if (copied_string != NULL)
free (copied_string);
I'd prefer these cleanups, but the change is now OK either way.
Thanks,
Roland