Re: some const char * trivia

[David Carlton <carlton@math.stanford.edu>]

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