Discussion about merging Go frontend

Richard Henderson rth@redhat.com
Sat Oct 30 08:07:00 GMT 2010


> +extern objfile_read *
> +objfile_open_read (int descriptor, off_t offset, const char *segment_name,
> +		   const char **errmsg, int *err);
...
> +extern objfile_write *
> +objfile_start_write (objfile_attributes *ATTRS, const char *segment_name,
> +		     const char **errmsg, int *err);

Mismatch in naming conventions?  I like the use of "release"
to clearly indicate non-closure of the FD.

> +  shdr_size = (cl == ELFCLASS32
> +	       ? sizeof (Elf32_External_Shdr)
> +	       : sizeof (Elf64_External_Shdr));
> +  memset (buf, 0, shdr_size);

Constant size memset is easier to optimize.  You might as well
zero all of BUF, even if we're not going to use it all.  The
slop between 32 and 64 is minimal.

> +  if (!objfile_internal_read (objfile->descriptor,
> +			      objfile->offset + eor->shoff + shdr_size,
> +			      shdrs,
> +			      shdr_size * (shnum - 1),
> +			      &errmsg, err))

Do we really want to keep re-reading section data for every section
lookup we do?  Can't we do this in objfile_open_read?

> +  set_32 (hdr + offsetof (struct external_scnhdr, s_flags),
> +	  (STYP_DATA | IMAGE_SCN_ALIGN_1BYTES | IMAGE_SCN_MEM_DISCARDABLE
> +	   | IMAGE_SCN_MEM_SHARED | IMAGE_SCN_MEM_READ));

You're not recording alignment in the coff object file?
IMAGE_SCN_ALIGN_<N>BYTES, 1 <= N <= 8192, are all defined
with a simple function in that nibble.


r~



More information about the Gcc-patches mailing list