Hi DJ
A) XNEWVEC
1) ./include/libiberty.h:
It appears that XNEWVEC() calls xmalloc which prints a message and calls xexit
if malloc fails.
#define XNEWVEC(T, N) ((T *) xmalloc (sizeof (T) * (N)))
/* Allocate memory without fail. If malloc fails, this will print a
message to stderr (using the name set by xmalloc_set_program_name,
if any) and then call xexit. */
extern void *xmalloc (size_t) ATTRIBUTE_MALLOC ATTRIBUTE_RETURNS_NONNULL;
2) ./libiberty/simple-object-xcoff.c :
It appears that XNEWVEC() was already called 2 times before we added a third
use of it, and still with NO check of return.
simple_object_xcoff_read_strtab (...)
{
...
strtab = XNEWVEC (char, strsize);
if (!simple_object_internal_read (sobj->descriptor, strtab_offset,
(unsigned char *) strtab, strsize, errmsg,
err))
...
simple_object_xcoff_find_sections (...)
{
...
scnbuf = XNEWVEC (unsigned char, scnhdr_size * ocr->nscns);
if (!simple_object_internal_read (sobj->descriptor,
sobj->offset + ocr->scnhdr_offset,
scnbuf, scnhdr_size * ocr->nscns, &errmsg,
err))
Thus, I think that we should continue to do what we did and do NOT check the
return of XNEWVEC() .
B) XDELETEVEC
1) ./include/libiberty.h:
#define XDELETEVEC(P) free ((void*) (P))
2) free() documentation : The free subroutine deallocates a ... If the Pointer
parameter is NULL, no action occurs.
So, yes, we check if (strtab == NULL) though there is no way that
XDELETEVEC(NULL) breaks something.
However, it is a classic programming style.
And the same programming style was used before we added our patch in
simple_object_xcoff_find_sections () :
/* The real section name is found in the string
table. */
if (strtab == NULL)
{
strtab = simple_object_xcoff_read_strtab (sobj,
&strtab_size,
&errmsg, err);
if (strtab == NULL)
{
XDELETEVEC (scnbuf);
return errmsg;
}
}
So our new code seems coherent with previous existing code.
Regards,
Cordialement,
Tony Reix
Bull - ATOS
IBM Coop Architect & Technical Leader
Office : +33 (0) 4 76 29 72 67
1 rue de Provence - 38432 Échirolles - France
www.atos.net
________________________________________
De : DJ Delorie [[email protected]]
Envoyé : mercredi 7 juin 2017 01:52
À : David Edelsohn
Cc : REIX, Tony; [email protected]; SARTER, MATTHIEU (ext);
[email protected]
Objet : Re: [PATCH,AIX] Enable libiberty to read AIX XCOFF
David Edelsohn <[email protected]> writes:
> This patch generally looks good to me -- it clearly is an incremental
> improvement. One of the libiberty maintainers, such as Ian, needs to
> approve the patch.
As AIX maintainer, I think you have the authority to approve patches
like this, which only affect your OS. I see no reason to reject the
patch myself, other than:
+ symtab = XNEWVEC (struct external_syment, ocr->nsyms * SYMESZ);
+ if (!simple_object_internal_read (sobj->descriptor,
There's no check to see if XNEWVEC succeeded.
Also, the use of XDELETEVEC is inconsistently protected with a "if (foo
!= NULL)" throughout, but passing NULL to XDELETEVEC (essentially,
free()) is allowed anyway, so this is only a stylistic issue, which I'm
not particularly worried about.
