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 wrote: >>>>>> "Eldar" == iam ahal 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 >