This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: some const char * trivia
- From: David Carlton <carlton at math dot stanford dot edu>
- To: Andrew Cagney <ac131313 at redhat dot com>
- Cc: gdb-patches at sources dot redhat dot com
- Date: 03 Jun 2003 14:18:26 -0700
- Subject: Re: some const char * trivia
- References: <3EDD07CD.1020900@redhat.com>
On Tue, 03 Jun 2003 16:40:45 -0400, Andrew Cagney <ac131313@redhat.com> said:
> I got slightly carried away with trying to eliminate some
> -Wwrite-strings errors. I'm trying to put the resultant mess on
> cagney_writestrings-20030508-branch.
Are you going to post patches, or are they too all-consuming/mindless
to matter? I wouldn't mind a brief description of changes you're
making. (E.g. what members of data structures are you turning into
const char *'s? All the name-like stuff?)
Anyways, looking at the patch to linespec.c, I was shocked to see that
the changes were that simple, but actually you've been misled by the
compiler. Consider set_flags (from mainline, not from your branch):
static void
set_flags (char *arg, int *is_quoted, char **paren_pointer)
{
char *ii;
int has_if = 0;
/* 'has_if' is for the syntax:
(gdb) break foo if (a==b)
*/
if ((ii = strstr (arg, " if ")) != NULL ||
(ii = strstr (arg, "\tif ")) != NULL ||
(ii = strstr (arg, " if\t")) != NULL ||
(ii = strstr (arg, "\tif\t")) != NULL ||
(ii = strstr (arg, " if(")) != NULL ||
(ii = strstr (arg, "\tif( ")) != NULL)
has_if = 1;
/* Temporarily zap out "if (condition)" to not confuse the
parenthesis-checking code below. This is undone below. Do not
change ii!! */
if (has_if)
{
*ii = '\0';
}
*is_quoted = (*arg
&& strchr (get_gdb_completer_quote_characters (),
*arg) != NULL);
*paren_pointer = strchr (arg, '(');
if (*paren_pointer != NULL)
*paren_pointer = strrchr (*paren_pointer, ')');
/* Now that we're safely past the paren_pointer check, put back " if
(condition)" so outer layers can see it. */
if (has_if)
*ii = ' ';
}
This really can modify the contents of *arg. But the compiler still
accepts it if you make arg a const char * (and paren_pointer too, I
guess), because the signature for strstr is inaccurate: if the man
page is to be believed, its signature is
char *strstr(const char *haystack, const char *needle);
but, of course, if 'haystack' really is a const char * then the return
value should also be treated as a const char *.
Sigh.
Actually, though, set_flags isn't so bad: it would be easy to rewrite
it to not modify what it points to. Something like this should work:
static void
set_flags (char *arg, int *is_quoted, char **paren_pointer)
{
char *ii;
int has_if = 0;
/* 'has_if' is for the syntax:
(gdb) break foo if (a==b)
*/
if ((ii = strstr (arg, " if ")) != NULL ||
(ii = strstr (arg, "\tif ")) != NULL ||
(ii = strstr (arg, " if\t")) != NULL ||
(ii = strstr (arg, "\tif\t")) != NULL ||
(ii = strstr (arg, " if(")) != NULL ||
(ii = strstr (arg, "\tif( ")) != NULL)
has_if = 1;
*is_quoted = (*arg
&& strchr (get_gdb_completer_quote_characters (),
*arg) != NULL);
*paren_pointer = strchr (arg, '(');
if (*paren_pointer != NULL && (!has_if || *paren_pointer < ii))
*paren_pointer = strrchr (*paren_pointer, ')');
if (has_if && *paren_pointer >= ii)
*paren_pointer = NULL;
}
That's just off the top of my head: I haven't tried to compile it.
Obviously one could work further to improve the clarity of the
function, as well.
There might be other places in linespec.c that play similar games,
too. I confess that I was under the impression that there was a place
where a string got modified for a longer amount of time: if the only
modification is in set_flags, then it's not nearly as bad as I feared.
David Carlton
carlton@math.stanford.edu