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 [d...@redhat.com] Envoyé : mercredi 7 juin 2017 01:52 À : David Edelsohn Cc : REIX, Tony; i...@golang.org; SARTER, MATTHIEU (ext); gcc-patches@gcc.gnu.org Objet : Re: [PATCH,AIX] Enable libiberty to read AIX XCOFF David Edelsohn <dje....@gmail.com> 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.