This is the mail archive of the mailing list for the binutils 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: PATCH: Print out bits of SOM headers

Hi Mark,

2004-11-08 Mark Mitchell <>

	* som.c (som_bfd_print_private_bfd_data): New function.
	(som_object_setup): Save the auxiliary header.  Don't assume that
	zero is an invalid entry point for a shared library.
	(som_object_p): Allocate the auxiliary header on the heap.

Could you make a couple of tidy-ups to the patch first please ?

+ static bfd_boolean som_bfd_print_private_bfd_data
+   PARAMS ((bfd *, void *));

We are using ISO C90 in binutils now, and even though this file has not yet been converted, I think that it is better that any new code added should follow this standard. Hence the PARAMS macro is no longer needed.

!   struct som_exec_auxhdr aux_hdr;
--- 2193,2203 ----
!   struct som_exec_auxhdr *aux_hdr = NULL;

My gut feeling here is that if you are changing the type of a variable you ought to change its name as well. Hence I would recommend changing aux_hdr to aux_hdr_ptr.

+ aux_hdr = ((struct som_exec_auxhdr *)
+ bfd_zalloc (abfd, + (bfd_size_type) sizeof (struct som_exec_auxhdr)));

bfd_zalloc returns a (void *) so there is no need for the cast to (struct som_exec_auxhdr *). Also, and this is just a personal preference, I think that it is safer to take the size of the pointer being allocated rather than the type that you know the pointer to be. That way if the pointer's type is ever changed you do not have to alter the code inside the alloc. ie, I would suggest that the above statement could be coded as:

aux_hdr_ptr = bfd_zalloc (abfd, (bfd_size_type) sizeof (* aux_hdr_ptr));

+ /* Display the SOM header. */
+ + static bfd_boolean
+ som_bfd_print_private_bfd_data (abfd, farg)
+ bfd *abfd;
+ void *farg;

ISO C90 again - please include the function's parameter types inside the parentheses.

+       auxhdr = &exec_header->som_auxhdr;
+       if (auxhdr->mandatory)
+ 	fprintf (f, "mandatory ");

For paranoia purposes I would suggest that you check for a NULL auxhdr before using it, in case the som_exec_auxhdr structure is corrupt.

With these changes made, please consider the patch approved.


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