From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 55592 invoked by alias); 3 May 2019 19:25:05 -0000 Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org Received: (qmail 55580 invoked by uid 89); 3 May 2019 19:25:04 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-10.8 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 spammy=presently, constituted X-HELO: userp2130.oracle.com Received: from userp2130.oracle.com (HELO userp2130.oracle.com) (156.151.31.86) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 03 May 2019 19:25:03 +0000 Received: from pps.filterd (userp2130.oracle.com [127.0.0.1]) by userp2130.oracle.com (8.16.0.27/8.16.0.27) with SMTP id x43JIqMu059847; Fri, 3 May 2019 19:24:57 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=from : to : cc : subject : references : date : in-reply-to : message-id : mime-version : content-type; s=corp-2018-07-02; bh=ZoylngqVhLwvdhgNRkolDnwAcEiKQfGEix+cnE5yZGM=; b=cjy+apVlT1oKXnrx8a7K+o8PrRCmaSKlnfdlVAGcSVq/5RZpHyAsaBqX/1JXhCP53yEm iDEK8FFU78Hq32bbnFsG1l3mkuUoqqDugqLB10s3fv4UikV7ds3UZKA+3BtTuj4FmaYy OrHY2Vc/NPQwCrqqKh9b297OJbOCHq6s/Atv9+hcklZ3mLav+b8gD1gvxZ5oizlffSoH NQ2ghvY0kMzOWVI+gwsoeJ53F6AHkIDwc/1OBdJLBWV1rk7V6sPAVaaZiaoeOQawGN81 3THniZULiQnUBVVueFOt6dl0cvw1rumOL2iQcC4XszGjOR72v1Ot7xklmYD8aTj3OOBb WA== Received: from aserp3020.oracle.com (aserp3020.oracle.com [141.146.126.70]) by userp2130.oracle.com with ESMTP id 2s6xhyrt7g-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 03 May 2019 19:24:57 +0000 Received: from pps.filterd (aserp3020.oracle.com [127.0.0.1]) by aserp3020.oracle.com (8.16.0.27/8.16.0.27) with SMTP id x43JOqlA162086; Fri, 3 May 2019 19:24:56 GMT Received: from aserv0122.oracle.com (aserv0122.oracle.com [141.146.126.236]) by aserp3020.oracle.com with ESMTP id 2s6xhhhvb8-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 03 May 2019 19:24:56 +0000 Received: from abhmp0011.oracle.com (abhmp0011.oracle.com [141.146.116.17]) by aserv0122.oracle.com (8.14.4/8.14.4) with ESMTP id x43JOtFh018701; Fri, 3 May 2019 19:24:55 GMT Received: from loom (/81.187.191.129) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Fri, 03 May 2019 12:24:55 -0700 From: Nick Alcock To: Nick Clifton Cc: binutils@sourceware.org Subject: Re: [PATCH 04/19] libctf: low-level list manipulation and helper utilities References: <20190430225706.159422-1-nick.alcock@oracle.com> <20190430225706.159422-5-nick.alcock@oracle.com> <5129234a-63db-cb60-145f-902babc3fb4d@redhat.com> Date: Fri, 03 May 2019 19:25:00 -0000 In-Reply-To: <5129234a-63db-cb60-145f-902babc3fb4d@redhat.com> (Nick Clifton's message of "Thu, 2 May 2019 17:04:09 +0100") Message-ID: <87sgtvuyre.fsf@esperi.org.uk> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2019-05/txt/msg00118.txt.bz2 On 2 May 2019, Nick Clifton said: > Hi Nick, > >> +#include > > This header is from the elfutils project right ? Given that, and the fact > that you are using the types from this header, why are you submitting this > code to the binutils project ? (Or maybe you are submitting it to both > projects - I have not checked). > > In particular the BFD library has its own ELF reading and writing functions > and its own headers defining the layout of ELF structures. Unfortunately > these headers do tend to conflict with the headers from the elfutils project, > whoch makes combining them problematical. See my response to Joseph. This is basically because Solaris has easily available and used it promiscuously: we are only using a few types that are always necessarily typedefs of types from , but of course that's glibc-specific so I suppose we can't rely on that either. But the binutils types seem... very far from ideal for my purposes here, terribly bfd-specific. Hence my suggestion (in an email that I hadn't written when you sent this) that I could simply copy the necessary types from the installed glibc headers into libctf. I don't know if that's too ugly to live. >> +/* Simple doubly-linked list append routine. This implementation assumes that >> + each list element contains an embedded ctf_list_t as the first member. >> + An additional ctf_list_t is used to store the head (l_next) and tail >> + (l_prev) pointers. The current head and tail list elements have their >> + previous and next pointers set to NULL, respectively. */ > > You knows this kind of code seems awfully familiar. I am sure that I have seen > it implemented in lots of different places... :-) Yes, but what else can we use? I tried using the stuff in and my eyes melted from all the CAPITAL LETTERS. :) >> +void >> +ctf_list_prepend (ctf_list_t * lp, void *new) > > I think that using "new" here might be a problem if you try to compile this > source file with a C++ compiler. True! I was only thinking in terms of using the headers with a C++ compiler... adjusted. However, there are a lot of other things we need to fix up before libctf is C++-ready: implicit conversions to/from void * are most of the problems, but we also use the %zi printf format in several places... >> +const char * >> +ctf_strraw (ctf_file_t *fp, uint32_t name) >> +{ >> + ctf_strs_t *ctsp = &fp->ctf_str[CTF_NAME_STID (name)]; > > My inner paranoia is screaming at code like this. Unless you > are certain that these functions cannot be called with out of > range parameters then I would strongly urge checking them before > using them. Possibly a nicer fix than explicitly checking is to change CTF_NAME_STID a bit to mask, from: #define CTF_NAME_STID(name) ((name) >> 31) to #define CTF_NAME_STID(name) (((name) >> 31) & 1) That should mask out all but the bottom bit, ensuring that even if someone manages to pass a 64-bit value with 30 1-bits to CTF_NAME_STID (... which cannot happen in ctf_strraw() as presently constituted, but a later change might break that assmuption), the result of CTF_NAME_STID() is always either 0 or 1. Thus it will always fit into ctf_str and never overwrite anything. ... or is this just piling too-cleverness on top of too-cleverness? (I've done that, provisionally, for v2.)