This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA/commit] Port GDB to powerpc-lynx178.


Yao,

Thanks for your comments.

Just one request: Would you mind cutting out the quoted test that
you are not replying to? It makes no sense to see emails where
you have to scroll pages and pages before seeing a one-line comment.

> >+  /* Stack pointer must be quadword aligned.  */
> >+  sp &= -16;
> 
> How about 'sp = align_down (sp, 16);'?
[...]
> >+      /* Add location required for the rest of the parameters.  */
> >+      space = (space + 15) & -16;
> 
> How about 'space = align_up (space, 16);'? and we may use
> 'align_up (XXX, 4)' somewhere else in this patch.

Why not indeed.

> >+  /* The only noticeable difference between Lynx178 XCOFF files and
> >+     AIX XCOFF files comes from the fact that there are no shared
> >+     libraries on Lynx178.  So if the number of import files is
> >+     different from zero, it cannot be a Lynx178 binary.  */
> >+  if (xcoff_get_n_import_files (abfd) != 0)
> >+    return GDB_OSABI_UNKNOWN;
> 
> As your comments said, we need the function returning a flag
> indicating 'the xcoff file has shared libraries or not', and looks
> the precise number of import files doesn't matter here.  I suggest
> that rename function 'xcoff_get_n_import_files' to
> 'xcoff_has_import_files'.

While your suggestion may be good enough for today, there might
come a day where someone will want the actual number of imports.
Since it does not cost anything to provide that information,
it would seem silly to spend any effort downgrading the function,
and take the risk of having to undo those changes someday.

Thanks,
-- 
Joel


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]