This is the mail archive of the binutils@sourceware.org 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 x64 SEH]: Support xdata/pdata for link-once code


On 09/15/2010 08:03 AM, Kai Tietz wrote:
> +  if (!name)
> +    name = "";

Name can never be null at this point.

> +  sname = xmalloc (strlen (base_name) + strlen (name) + 1);
> +
> +  /* Create segment.  */
> +  sprintf (sname, "%s%s", base_name, name);

 sname = concat (base_name, name, NULL);

from libiberty.

> +  r = (struct seh_seg_list *)
> +    xmalloc (sizeof (struct seh_seg_list) + strlen (name) + 1);

The +1 is already included via "char name[1]" in the struct.

Keep hold of the strlen result and you can use memcpy later
instead of strcpy.

> +  if ((item = seh_hash_find (name)) == NULL)
>      {
> +      item = alloc_pxdata_item (make_pxdata_seg (cseg, name), 0, name);
> +
> +      seh_hash_insert (item->seg_name, item);
>      }
>    else
> +    free (name);

Why isn't NAME freed along both paths?  Alternately, why
play with the variable sized allocation in alloc_pxdata_item?

Also, I really don't like assignments within conditionals.
Pull that seh_hash_find out into a separate statement.


r~


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