* 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