From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19117 invoked by alias); 7 Jun 2017 13:16:38 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 10518 invoked by uid 89); 7 Jun 2017 13:16:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: =?ISO-8859-1?Q?No, score=-2.3 required=5.0 tests=AWL,BAYES_00,SPF_PASS,T_RP_MATCHES_RCVD,UNPARSEABLE_RELAY autolearn=ham version=3.3.2 spammy=cordialement, envoy, objet, envoy=e9?= X-HELO: smtppost.atos.net Received: from smtppost.atos.net (HELO smtppost.atos.net) (193.56.114.166) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 07 Jun 2017 13:16:32 +0000 Received: from mail2-ext.my-it-solutions.net (mail2-ext.my-it-solutions.net) by smarthost6.atos.net with smtp (TLS: TLSv1/SSLv3,256bits,ECDHE-RSA-AES256-SHA) id 7042_1b8c_fabf8d3e_f696_4112_9b56_d703ed2d8419; Wed, 07 Jun 2017 15:16:22 +0200 Received: from mail2-int.my-it-solutions.net ([10.92.32.13]) by mail2-ext.my-it-solutions.net (8.15.2/8.15.2) with ESMTPS id v57DGLCI029217 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 7 Jun 2017 15:16:21 +0200 Received: from FRAUVJ99ET2MSX.ww931.my-it-solutions.net ([172.23.231.42]) by mail2-int.my-it-solutions.net (8.15.2/8.15.2) with ESMTPS id v57DGK7o030341 (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=FAIL); Wed, 7 Jun 2017 15:16:21 +0200 Received: from FRCRPVV9EX4MSX.ww931.my-it-solutions.net ([169.254.7.172]) by FRAUVJ99ET2MSX.ww931.my-it-solutions.net ([172.23.231.42]) with mapi id 14.03.0339.000; Wed, 7 Jun 2017 15:16:20 +0200 From: "REIX, Tony" To: DJ Delorie , David Edelsohn CC: "iant@golang.org" , "SARTER, MATTHIEU (ext)" , "gcc-patches@gcc.gnu.org" Subject: RE:[PATCH,AIX] Enable libiberty to read AIX XCOFF Date: Wed, 07 Jun 2017 13:16:00 -0000 Message-ID: References: (message from David Edelsohn on Tue, 6 Jun 2017 19:25:09 -0400), In-Reply-To: Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-SW-Source: 2017-06/txt/msg00434.txt.bz2 Hi DJ A) XNEWVEC 1) ./include/libiberty.h: It appears that XNEWVEC() calls xmalloc which prints a message and calls xe= xit 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 thi= rd use of it, and still with NO check of return. simple_object_xcoff_read_strtab (...) { ... strtab =3D XNEWVEC (char, strsize); if (!simple_object_internal_read (sobj->descriptor, strtab_offset, (unsigned char *) strtab, strsize, errm= sg, err)) ... simple_object_xcoff_find_sections (...) { ... scnbuf =3D XNEWVEC (unsigned char, scnhdr_size * ocr->nscns); if (!simple_object_internal_read (sobj->descriptor, sobj->offset + ocr->scnhdr_offset, scnbuf, scnhdr_size * ocr->nscns, &errm= sg, err)) Thus, I think that we should continue to do what we did and do NOT check th= e 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 Poi= nter parameter is NULL, no action occurs. So, yes, we check if (strtab =3D=3D NULL) though there is no way that X= DELETEVEC(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 =3D=3D NULL) { strtab =3D simple_object_xcoff_read_strtab (sobj, &strtab_size, &errmsg, err); if (strtab =3D=3D 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 =C9chirolles - France www.atos.net ________________________________________ De : DJ Delorie [dj@redhat.com] Envoy=E9 : mercredi 7 juin 2017 01:52 =C0 : David Edelsohn Cc : REIX, Tony; iant@golang.org; SARTER, MATTHIEU (ext); gcc-patches@gcc.g= nu.org Objet : Re: [PATCH,AIX] Enable libiberty to read AIX XCOFF David Edelsohn 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 =3D 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 !=3D 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.