[patch] GDB 7.2: new feature for "backtrace" that cuts path to file (remain filename)

[iam ahal]

Copyright assignment by [gnu.org #703515] is completed.
I've corrected some pieces of the patch by your notes.
Also, I've attached the change log.

Please, check it.

Thank you.

~Eldar.

On Wed, Aug 3, 2011 at 9:44 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Eldar" == iam ahal <hal9000ed2k@gmail.com> writes:
>
> Tom> I forget (I never keep records of this, I think perhaps I should) -- did
> Tom> we get you started on the copyright assignment paperwork?
>
> Eldar> Not yet, because my previous patches is very different. I mean the
> Eldar> different files was changed and I don't know which files I need to
> Eldar> change in the next patch.
> Eldar> I hope some people here can accept implementation in this last patch.
> Eldar> After I can start the copyright assignment.
>
> It is best to start early.  Sometimes it can take quite a while, and in
> the meantime your patch will not go in.
>
> The list of files does not really matter, since the assignment is for
> past and future changes.  You can ask the copyright clerk for
> clarification on this point if you need it.
>
> I think the patch is basically fine, just a few nits.
>
> Eldar> +static int backtrace_skip_compile;
> Eldar> +static void
>
> Tom> Newline between these two lines.
>
> Eldar> I'm not sure that's right because I see that there's no newline in
> Eldar> this case (if you look at other places related 'set backtrace ...').
>
> Yeah, I think a newline is better, though.
>
> Eldar> +char *
> Eldar> +get_display_filename_from_sal (struct symtab_and_line *sal)
> Tom>
> Tom> Should have an introductory comment before this function.
>
> Eldar> Comment was added in 'frame.h'
>
> Ok, thanks.
> Add a comment saying to see the documentation in frame.h.
>
> Eldar> +static const char filename_display_without_comp_dir[] = "without-compile-dir";
>
> I think spelling out "directory" would be better.
>
> Eldar> +  const char *filename = sal->symtab->filename;
> Eldar> +  const char *dirname = sal->symtab->dirname;
> Eldar> +  size_t flen = strlen (filename);
> Eldar> +  size_t dlen = strlen (dirname);
>
> This will crash if filename==NULL or dirname==NULL.
>
> Eldar> +  if (filename_display_string == filename_display_without_comp_dir
> Eldar> +      && filename && dirname && dlen <= flen
> Eldar> +      && !FILENAME_NCMP (filename, dirname, dlen))
>
> This test is not correct, in that it will match "/tmp/x" with "/tmp/xaaa".
> I think it needs an additional check for a separator.
>
> Eldar> diff -rup gdb-7.2-orig/include/filenames.h gdb-7.2/include/filenames.h
>
> Changes to libiberty have to go to gcc-patches.
> I don't think you actually need this change, though, so it would be
> simpler and quicker if you left it out.
>
> Eldar> +extern int filename_ncmp (const char *s1, const char *s2, size_t n);
>
> This is already declared in the trunk filenames.h.
> Be sure to always write patches against the trunk, not something older.
>
> Tom
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: gdb-7.3.1-filename-display-v1.patch
Type: text/x-patch
Size: 6012 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20111030/b3c1044b/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ChangeLog
Type: application/octet-stream
Size: 839 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20111030/b3c1044b/attachment.obj>