Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* some const char * trivia
@ 2003-06-03 20:40 Andrew Cagney
  2003-06-03 21:18 ` David Carlton
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Cagney @ 2003-06-03 20:40 UTC (permalink / raw)
  To: gdb-patches

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.  Anyway, I learnt two things:

- the cli callbacks cause much grief
An incremental approach where a new call back function signature (that 
took a const char *) was introduced might make that transition easier.

- strtol(const char *, char **, int base) is a pain
Perhaphs something like:
LONGEST strtolongest (const char *b, const char **p, int base);
would be useful.

Andrew


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: some const char * trivia
  2003-06-03 20:40 some const char * trivia Andrew Cagney
@ 2003-06-03 21:18 ` David Carlton
  2003-06-03 22:08   ` Andrew Cagney
  0 siblings, 1 reply; 3+ messages in thread
From: David Carlton @ 2003-06-03 21:18 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: some const char * trivia
  2003-06-03 21:18 ` David Carlton
@ 2003-06-03 22:08   ` Andrew Cagney
  0 siblings, 0 replies; 3+ messages in thread
From: Andrew Cagney @ 2003-06-03 22:08 UTC (permalink / raw)
  To: David Carlton; +Cc: gdb-patches

> 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?)

The doco in the cli.  It's where it tried to free it that I got scared :-)

> 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):

I was using an iterative process:

	- compile with -Wwrite-strings
	- change one function signature to const char *
	- compile with -Wno-write-strings
	- fix up the parameter passing/assignment mess
	- repeat.

And gave up when things got really messy.  So don't take the branch too 
seriously.  I was carefully avoiding linespec.

Andrew




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2003-06-03 22:08 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-06-03 20:40 some const char * trivia Andrew Cagney
2003-06-03 21:18 ` David Carlton
2003-06-03 22:08   ` Andrew Cagney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox