Re: [patch] Assert when 'break' with no arguments

[Joel Brobecker <brobecker@adacore.com>]

> ChangeLog:
> 2012-02-14  Aleksandar Ristovski  <aristovski@qnx.com>
> 
>        * frame.c (find_frame_sal): Initialise sal->pspace field from
> frame data.
>        * stack.c (set_last_displayed_sal): Perform sanity check of the data
>        passed in, in particular, validate that PSPACE is not NULL if
> requesting
>        valid last_displayed_* data.
> 
> 
> Test suite ChangeLOg:
> 2012-02-14  Aleksandar Ristovski  <aristovski@qnx.com>
> 
>     * gdb.base/break-inline.exp: New test.
>     * gdb.base/break-inline.c: New test.

I haven't looked at the validity of the patch (Pedro has a better
understanding of this area, for instance), but I still noticed some
trivial deviations from the GNU Coding Standards.

Your ChangeLog entry, for instance. Lines should be folded at 70 chars
(hard limit is 80 chars). Other violations highlighted below:

> +      /* Set pspace with frame's pspace */

End the sentence with a period (and two spaces before the '*/').

> +  if (valid && pspace == NULL) {
> +	warning(_("Trying to set NULL pspace."));
> +  }

Wrong formatting for first and second line.

> +  if (valid && pspace == NULL)
> +	last_displayed_sal_valid = 0;

Ditto for the second line (indentation is 2 spaces).

>    last_displayed_symtab = symtab;
>    last_displayed_line = line;
> +
>  }

Why are you adding an empty line here?

> #   Copyright 2012 Free
> #   Software Foundation, Inc.

Missing (C), and please join the two lines. If you copied some of
the testcase from another testcase, then you need to preserve the
original copyright years, I think.

> /* This testcase is part of GDB, the GNU debugger.
> 
>    Copyright 2012 Free Software
>    Foundation, Inc.

Same as above, please add the missing (C).

> #include <stdio.h>
> static int g;
> static inline void foo(void)
> {
>   g = 42;
>   printf("%d\n", g);
> }
> int main(int argc, char *argv[])
> {
>   foo();
>   return g;
> }

Is there a way to trigger the same problem without the dependency
on stdio.h? Many systems do not provide it (bareboard). I would
think that all you need is to define a function that has the same
profile as printf, no?

-- 
Joel