From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from zmcc-2-mx.zmailcloud.com (zmcc-2-mx.zmailcloud.com [52.37.197.7]) by sourceware.org (Postfix) with ESMTPS id 7F25738708C3 for ; Tue, 22 Sep 2020 11:42:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 7F25738708C3 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=symas.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=hyc@symas.com Received: from zmcc-2.zmailcloud.com (zmcc-2-mta-1.zmailcloud.com [146.148.52.56]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by zmcc-2-mx.zmailcloud.com (Postfix) with ESMTPS id 9BF3040665; Tue, 22 Sep 2020 06:42:38 -0500 (CDT) Received: from zmcc-2.zmailcloud.com (localhost [127.0.0.1]) by zmcc-2-mta-1.zmailcloud.com (Postfix) with ESMTPS id 2580FCCD90; Tue, 22 Sep 2020 06:42:38 -0500 (CDT) Received: from localhost (localhost [127.0.0.1]) by zmcc-2-mta-1.zmailcloud.com (Postfix) with ESMTP id 0CC69C2B3B; Tue, 22 Sep 2020 06:42:38 -0500 (CDT) DKIM-Filter: OpenDKIM Filter v2.10.3 zmcc-2-mta-1.zmailcloud.com 0CC69C2B3B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=symas.com; s=37C7994C-28CA-11EA-A30F-68F90BB9D764; t=1600774958; bh=g6iI7aT8cr4gSrQFytttBAIhib9LnhCxzTkvJ4M7nOs=; h=To:From:Message-ID:Date:MIME-Version; b=pN0SB3sGE3tz+l6d1u0uigP8B6hLnMyVkfh4vmI0Zi+9Sn7bUViLMasJ/xQSE0OrT nocqTZVI9eirZ0LzvODSABWE/tDoxXi6YHbVrVTj5oRhPv95140yWHjWDCSzymuOw9 /TyVopH4nlBcBZL1goaUgsW90gkONL/OFpbWNA4GSN3BGT5tdGcj8XlDqyMxZrDtJC CEXwGWoHecnKN3dhHXqd0T8NWecWzyGLelDl6oitfUEJbJxUa/GQeaTDAhPzpuNtVL KQSXdcpz+uTpZ5TZJHyzclZCzfNBbNXK5Vz12yCYCQJuLDZ7G9nqq3IZrG483uqnv+ hRr4RcER8TGHQ== Received: from zmcc-2.zmailcloud.com ([127.0.0.1]) by localhost (zmcc-2-mta-1.zmailcloud.com [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id L-fQJFtbAi3f; Tue, 22 Sep 2020 06:42:37 -0500 (CDT) Received: from [192.168.1.155] (unknown [84.203.28.168]) by zmcc-2-mta-1.zmailcloud.com (Postfix) with ESMTPSA id 5A0CAC0E9B; Tue, 22 Sep 2020 06:42:36 -0500 (CDT) Subject: Re: dependency list for static libraries To: Nick Clifton Cc: binutils@sourceware.org References: <53b8973b-40a4-2550-3307-66d7f13707d5@symas.com> <64fe82bd-9c00-b232-98d2-f46182fb16ba@symas.com> <9889c54b-4dd3-2275-6621-c2391cfd268d@redhat.com> From: Howard Chu Message-ID: Date: Tue, 22 Sep 2020 12:42:34 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Firefox/60.0 SeaMonkey/2.53.3 MIME-Version: 1.0 In-Reply-To: <9889c54b-4dd3-2275-6621-c2391cfd268d@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, KAM_SHORT, NICE_REPLY_A, SPF_HELO_NONE, SPF_PASS, TXREP, URIBL_RED autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 22 Sep 2020 11:42:41 -0000 Nick Clifton wrote: > Hi Howard, > >> Here's a proposed patch to ar to implement the first half of the solution: >> storing the dependencies into a static library. > > Some comments on the patch: Thanks for the feedback. > > * You need to add documentation of the new feature to binutils/doc/bin.texi > and also a note to binutils/NEWS. OK, will add. > > * Whilst single letter options are the convention for ar, it would also be > good if the GNU convention of providing a more verbose alternative option > name were used to. Eg --record-libdeps=. > > Whilst on the subject of options, you should add some error checking for > the presence of multiple L options. Or maybe just allow multiple L options to be concatenated together? > * There are several places where you call bfd library functions but do not > check the return values. This is a bad idea. Always be paranoid and > check to ensure that a function call has succeeded. Ok. > * It is not clear to me why you create a binary bfd for Libdeps_bfd but > then convert it to a plugin type bfd. Can you explain what you are > doing here ? This was a major hassle, I should have commented it. The bfd gets created with type "plugin", and that refuses the bfd_bwrite() call. (Just fails.) The write only succeeded if I set it to "binary" type first. But then, trying to add this bfd to the archive failed unless I changed the type back to "plugin." All of this was unexpected, none of the docs for make_writable or make_readable mentioned anything about these incompatibilities. > * The change to the code to call ar_emul_replace() inside replace_members() > looks wrong to me. The current code will try to replace all of the entries > on the files_to_move list, and will set changed to TRUE if any of the > replacements succeeds. The patched code will changed to FALSE if any > replacement fails, even if earlier ones succeeded. No, that's not correct. The patched code ORs in the result, so it will not change any previous success into a failure. > Plus once one > replacement has failed, future successful replacements will not be removed > from the archive chain. Not sure that's the case either. > * There are some formatting issues as well. These are minor, but it is > helpful to ensure that the codebase uses a uniform style. Please check > out the GNU Coding Standards for more information: The code used exactly the same indentation as the rest of the source file. I can check it again, but ... this is not my first time contributing to GNU projects... > > https://www.gnu.org/prep/standards/standards.html > > * It would also be really helpful if you could create a test for the > binutils testsuite that checks this new functionality. OK. Any suggestions on what exactly to check? >> It looks like I may be able >> to use the linker plugin facility to handle the ld side of things. But it's >> not clear to me that it won't clash with other plugins. I.e., if one plugin >> claims an archive file, will that prevent other plugins from being able to >> process it? > > I am not sure that this has been done before, but I think that you are right - > once a file/archive has been claimed by one plugin, it cannot be claimed by > another. It may however be possible for your plugin to process an archive > but not claim it. This is just an idea - I have not tested it - and it might > mean that the ordering of plugins on the command line becomes important... > > Cheers > Nick > > > -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/