New ARI web page, generated using script inside CVS tree in gdb/contrib/ari directory

New ARI web page, generated using script inside CVS tree in gdb/contrib/ari directory

[Pierre Muller]

  Recently, an in-tree version
of the scripts used to generate the ARI web-page was
added to gdb CVS repository.

The original ARI page is at:
http://sourceware.org/gdb/current/ari/

while the new page is temporarily at:
http://sourceware.org/gdb/current/ari/test/

The main advantage of this in-tree version is that it is easier
to adapt to changes inside CVS tree.

If you look for instance in the first link,
you will see that there are 13 regressions
listed in the 'Critical' section.
  This list was easily reduced to 4 in the second link,
because lots of those regression are simply related
to changes in the sources, especially code moves from 
gdb to gdb/common sub-directory.

  Out of the 4 remaining critical issues

Critical
Things previously eliminated but returned. This should always be empty.

BUG	Total	Description
1) strerror	10	Do not use strerror(), instead use safe_strerror()
2) xasprintf	2	Do not use xasprintf(), instead use xstrprintf
3) stat.h	1	Do not include stat.h or sys/stat.h, instead include
gdb_stat.h
4) wait.h	1	Do not include wait.h or sys/wait.h, instead include
gdb_wait.h


2) will be solved as soon as the [RFA] Fix New ARI warning Tue Nov  6
01:58:48 UTC 2012
gets approved.

3) and 4) are all related to the fact that 
gdb_stat.h and gdb_wait.h headers are still in gdb directory,
which force the use of direct system include headers in sources in
gdb/common
sub-directory.
  Those can probably be solved easily by moving the two headers also
to gdb/common.

  But before sending a patch, I would like to know if this is the right
direction to go to...

1) is a little bit more tricky because safe_strerror is declared in utils.h
header,
but implemented in two files:
posix-hdep.c and mingw-hdep.c

  Should I extract the declaration into a
gdb/common/gdb_strerror.h
and extract the two functions
into gdb/common/posix-strerror.c and gdb/common/mingw-strerror.c?


  Finally, a few broader questions regarding ARI:

Should we add rules about
  gdb_XXX.h headers
stating that if gdb_XXX.h header exists and shadows an existing
system header, no gdb C source file should include XXX.h system header
directly.

  I would tend to find such a rule logical, but it
would probably force us to more most of these
headers into gdb/common subdirectory, and I don't really know if this
trend is really accepted by the global maintainers.

  Also, I would find as a logical consequence that also
gdbserver subdirectory should follow the ARI rules.
  This is done quite easily by removing the 
  -name gdbserver -prune -o 
line from gdb_find.sh script in gdb/contrib./ari
but is a rather important change that should be discussed fully.

Any comments most welcome,

Pierre Muller
as ARI maintainer.






Pierre Muller
as ARI maintainer

Re: New ARI web page, generated using script inside CVS tree in gdb/contrib/ari directory

[Joel Brobecker]

> 3) stat.h	1	Do not include stat.h or sys/stat.h, instead include
> gdb_stat.h
> 4) wait.h	1	Do not include wait.h or sys/wait.h, instead include
> gdb_wait.h
[...]
> 3) and 4) are all related to the fact that gdb_stat.h and gdb_wait.h
> headers are still in gdb directory, which force the use of direct
> system include headers in sources in gdb/common sub-directory.  Those
> can probably be solved easily by moving the two headers also to
> gdb/common.

I had a look at gdb_stat.h and gdb_wait.h, and I think it would be
fine to move them over to gdb/common.

But perhaps we should also look at possibly importing gnulib's stat
and sys_wait modules? Not a trivial change in the sense that it could
be not equivalent to what we have now, and thus have unintended
consequences; but perhaps worth a shot. I think that gdb_stat.h and
all other such headers were GDB's own way of doing what gnulib does
in general.

> 1) is a little bit more tricky because safe_strerror is declared in utils.h
> header,
> but implemented in two files:
> posix-hdep.c and mingw-hdep.c
> 
>   Should I extract the declaration into a
> gdb/common/gdb_strerror.h
> and extract the two functions
> into gdb/common/posix-strerror.c and gdb/common/mingw-strerror.c?

This sounds good to me. There is the option of putting all
implementation into one single file, the way we do for some
of our code, but I don't think it would be particularly simpler
to do that way, since the current code has it separated in 2 files.

In terms of the name, perhaps we should make it more explicit that
this is not strerror, but safe_strerror?

> Should we add rules about
>   gdb_XXX.h headers
> stating that if gdb_XXX.h header exists and shadows an existing
> system header, no gdb C source file should include XXX.h system header
> directly.

I would think so - but we should also look at gnulib, to see if
it couldn't just be replaced by gnulib's equivalent.

>   Also, I would find as a logical consequence that also
> gdbserver subdirectory should follow the ARI rules.
>   This is done quite easily by removing the 
>   -name gdbserver -prune -o 
> line from gdb_find.sh script in gdb/contrib./ari
> but is a rather important change that should be discussed fully.

Right. I think it would be a helpful, but I'll let Pedro comment
on that one.

-- 
Joel

Re: New ARI web page, generated using script inside CVS tree in gdb/contrib/ari directory

[Pedro Alves]

On 11/12/2012 06:07 PM, Joel Brobecker wrote:

>> 1) is a little bit more tricky because safe_strerror is declared in utils.h
>> header,
>> but implemented in two files:
>> posix-hdep.c and mingw-hdep.c
>>
>>   Should I extract the declaration into a
>> gdb/common/gdb_strerror.h
>> and extract the two functions
>> into gdb/common/posix-strerror.c and gdb/common/mingw-strerror.c?
> 
> This sounds good to me. There is the option of putting all
> implementation into one single file, the way we do for some
> of our code, but I don't think it would be particularly simpler
> to do that way, since the current code has it separated in 2 files.
> 
> In terms of the name, perhaps we should make it more explicit that
> this is not strerror, but safe_strerror?

strerror sounds like something gnulib might have a replacement for
too.  Then we could get rid of safe_strerror, and just use strerror
everywhere.

-- 
Pedro Alves

run the ARI on gdbserver too?

[Pedro Alves]

On 11/12/2012 06:07 PM, Joel Brobecker wrote:
>> >   Also, I would find as a logical consequence that also
>> > gdbserver subdirectory should follow the ARI rules.
>> >   This is done quite easily by removing the 
>> >   -name gdbserver -prune -o 
>> > line from gdb_find.sh script in gdb/contrib./ari
>> > but is a rather important change that should be discussed fully.
> Right. I think it would be a helpful, but I'll let Pedro comment
> on that one.

I agree that it's a logical step.  I think we'll see a lot of
hits that don't actually point at issues that are problems in
practice (due to the fact lots of gdbserver code is host/native
code that makes assumptions on the environment its being built
for, like bits of native code in gdb does), but it sounds nevertheless
a good idea, considering we'll want to share more and more between
gdb and gdbserver.

-- 
Pedro Alves

Re: New ARI web page, generated using script inside CVS tree in gdb/contrib/ari directory

[Joel Brobecker]

> strerror sounds like something gnulib might have a replacement for
> too.  Then we could get rid of safe_strerror, and just use strerror
> everywhere.

I had a look at that option, and I'm not sure whether it would work on
Windows. But maybe I looked too quickly. If it works on windows, I am
definitely in favor of that too.

-- 
Joel

Re: New ARI web page, generated using script inside CVS tree in gdb/contrib/ari directory

[Pedro Alves]

On 11/12/2012 06:38 PM, Joel Brobecker wrote:
>> strerror sounds like something gnulib might have a replacement for
>> too.  Then we could get rid of safe_strerror, and just use strerror
>> everywhere.
> 
> I had a look at that option, and I'm not sure whether it would work on
> Windows. But maybe I looked too quickly. If it works on windows, I am
> definitely in favor of that too.

Hmm, doesn't look like a full replacement.  Ours uses FormatMessage
for errors unknown to the native strerror, while gnulib's
returns "(undocumented error %d)".  Don't know if gnulib might be
interested in our version, though to me it sounds like our version is
more useful for Windows ports; it might be worth it to ask them and
contribute it.

-- 
Pedro Alves

Re: New ARI web page, generated using script inside CVS tree in gdb/contrib/ari directory

[Joel Brobecker]

> Hmm, doesn't look like a full replacement.  Ours uses FormatMessage
> for errors unknown to the native strerror, while gnulib's
> returns "(undocumented error %d)".  Don't know if gnulib might be
> interested in our version, though to me it sounds like our version is
> more useful for Windows ports; it might be worth it to ask them and
> contribute it.

I think they might. I've recently contributed some changes to errno,
and it didn't take long to converge and get something checked in.

-- 
Joel

RE: run the ARI on gdbserver too?

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-owner@sourceware.org [mailto:gdb-owner@sourceware.org] De la part
> de Pedro Alves
> Envoyé : lundi 12 novembre 2012 19:38
> À : Joel Brobecker
> Cc : Pierre Muller; gdb@sourceware.org
> Objet : run the ARI on gdbserver too?
> 
> On 11/12/2012 06:07 PM, Joel Brobecker wrote:
> >> >   Also, I would find as a logical consequence that also
> >> > gdbserver subdirectory should follow the ARI rules.
> >> >   This is done quite easily by removing the
> >> >   -name gdbserver -prune -o
> >> > line from gdb_find.sh script in gdb/contrib./ari
> >> > but is a rather important change that should be discussed fully.
> > Right. I think it would be a helpful, but I'll let Pedro comment
> > on that one.
> 
> I agree that it's a logical step.  I think we'll see a lot of
> hits that don't actually point at issues that are problems in
> practice (due to the fact lots of gdbserver code is host/native
> code that makes assumptions on the environment its being built
> for, like bits of native code in gdb does), but it sounds nevertheless
> a good idea, considering we'll want to share more and more between
> gdb and gdbserver.

  This was one of the reasons 
of my old proposition to distinguish between
native and general files.
http://sourceware.org/ml/gdb-patches/2011-03/msg01087.html

  Using things like 'long long' type or '%ll' or '%p'
is not really a problem for native files, but
can become one for general files as the size of the
fields might not match was is expected.


Pierre Muller

Re: New ARI web page, generated using script inside CVS tree in gdb/contrib/ari directory

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> But perhaps we should also look at possibly importing gnulib's stat
Joel> and sys_wait modules? Not a trivial change in the sense that it could
Joel> be not equivalent to what we have now, and thus have unintended
Joel> consequences; but perhaps worth a shot. I think that gdb_stat.h and
Joel> all other such headers were GDB's own way of doing what gnulib does
Joel> in general.

FWIW I tend to favor more use of gnulib in gdb.  In general I think the
pros outweigh the cons; especially since it seems reasonably easy to get
fixes into gnulib, and because importing a new gnulib snapshot is also
simple.

The pros seem to be -- shared development, good documentation, and
letting the main gdb source use standard headers and standard functions.

I think the primary con is that a gnulib module may have a bug, and then
we have to fix it elsewhere first.  This doesn't seem to be a major
problem.

Tom

Re: New ARI web page, generated using script inside CVS tree in gdb/contrib/ari directory

[Joel Brobecker]

> FWIW I tend to favor more use of gnulib in gdb.  In general I think the
> pros outweigh the cons; especially since it seems reasonably easy to get
> fixes into gnulib, and because importing a new gnulib snapshot is also
> simple.

Same for me.

> I think the primary con is that a gnulib module may have a bug, and then
> we have to fix it elsewhere first.  This doesn't seem to be a major
> problem.

I think it's actually the same as having to fix the problem in GDB
because the gnulib maintainers have been pretty responsive with
my patches so far.

-- 
Joel

RE: run the ARI on gdbserver too?

[Pierre Muller]

  Hi,

  I was wondering if I could
commit as obvious ARI fixes to gdbserver 
directory, so that, when we finally add
gdbserver to the ARI directory list
(in fact remove the line pruning the directory in contrib./ari/gdb_find.sh
script)
we would get a lower increase of the number of problems...

Pierre 
as ARI maintainer


> -----Message d'origine-----
> De : gdb-owner@sourceware.org [mailto:gdb-owner@sourceware.org] De la part
> de Pedro Alves
> Envoyé : lundi 12 novembre 2012 19:38
> À : Joel Brobecker
> Cc : Pierre Muller; gdb@sourceware.org
> Objet : run the ARI on gdbserver too?
> 
> On 11/12/2012 06:07 PM, Joel Brobecker wrote:
> >> >   Also, I would find as a logical consequence that also
> >> > gdbserver subdirectory should follow the ARI rules.
> >> >   This is done quite easily by removing the
> >> >   -name gdbserver -prune -o
> >> > line from gdb_find.sh script in gdb/contrib./ari
> >> > but is a rather important change that should be discussed fully.
> > Right. I think it would be a helpful, but I'll let Pedro comment
> > on that one.
> 
> I agree that it's a logical step.  I think we'll see a lot of
> hits that don't actually point at issues that are problems in
> practice (due to the fact lots of gdbserver code is host/native
> code that makes assumptions on the environment its being built
> for, like bits of native code in gdb does), but it sounds nevertheless
> a good idea, considering we'll want to share more and more between
> gdb and gdbserver.
> 
> --
> Pedro Alves

Re: run the ARI on gdbserver too?

[Pedro Alves]

On 16-11-2012 16:02, Pierre Muller wrote:

>   I was wondering if I could
> commit as obvious ARI fixes to gdbserver 
> directory, so that, when we finally add
> gdbserver to the ARI directory list
> (in fact remove the line pruning the directory in contrib./ari/gdb_find.sh
> script)
> we would get a lower increase of the number of problems...

The obvious rule as described in MAINTAINERS also applies to gdbserver.
There's no particular "ARI fixes are obvious" golden rule.  If any given
individual change is obvious, then go right ahead and check it in.  If not,
post first.

Thanks.

-- 
Pedro Alves