From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 99732 invoked by alias); 4 Jul 2016 14:27:39 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 99717 invoked by uid 89); 4 Jul 2016 14:27:38 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 spammy=vision, beside, Hx-languages-length:2149, million X-HELO: mail-qk0-f174.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:from:organization :message-id:date:user-agent:mime-version:in-reply-to :content-transfer-encoding; bh=3fv136s4RtJUoU1Tq9e5RIXZC4hKg4HCIAjdH4FhDs8=; b=RGPCmAa//1dhYSwh9dBB80KP18geXRolmasjjn5UjW5Q+e8nlYAIbuL9tgzJKeFzyX +ToZiC8kiBZW0Pg8iBPdZaLAodeNyIU5NgfNh/MlWMffbr4NI/usgY6Rog70bWJREDb4 HNFkbi01bo83VKJax3VPfwdFeLADXr5jMLOu7sfzzFEIUkSIK3Wo2jn4/Dc1dq7xf4Ga UWJ5KISeK5ikh5otmYNTPD5LEikAf+Ub00CuA7X+44lTGqZoaOsKOOw3i0eY/x3QkbqB zY7ZyG/QzL96YLf2g54uApAJ1CJZ8NSDqVtLBUDgfmwzmh2te5kGq2hMVQNgLasgLDS5 qcQg== X-Gm-Message-State: ALyK8tJNvRHpdUSqY23HttSI+z8Iohi578vo4k4xRnWTDPAqJtRdsNzWsRzv16+nPqwbSkku X-Received: by 10.55.94.135 with SMTP id s129mr4394890qkb.139.1467642454084; Mon, 04 Jul 2016 07:27:34 -0700 (PDT) Subject: Re: Consensus on unit tests? To: Florian Weimer , GNU C Library References: <92609f90-7a53-a455-ca72-b9baae224a38@redhat.com> From: Carlos O'Donell Message-ID: <84127229-3bc2-8057-e1be-e4a78889fdbe@redhat.com> Date: Mon, 04 Jul 2016 14:27:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2016-07/txt/msg00100.txt.bz2 On 07/01/2016 02:43 AM, Florian Weimer wrote: > On 06/25/2016 01:59 AM, Carlos O'Donell wrote: > >> I would like to remove the superfluous condition and add a unit >> test for all the cases that define the way the interface should >> behave. >> >> Since Florian asked for pretty diagrams, I have included them. > > I can't quote your patch due to the way it is included in the message > (inline text after the signature). I can certainly adjust the way I inline my messages if that helps. Out of curiosity, what MUA are you using? > What's unclear based on the documentation if the address has to fall > in the range covered by the link map (i.e., if there are indeed only > three cases, or five). If there is indeed a precondition that the > address is in some special range, you should add it to the comment. There is no precondition that I am aware of. I have clarified the new patch to say "loadable segment" where I previously said "segment" to make it more clear. There are some "optimizations" going on here, and l_contiguous is used to avoid calling _dl_addr_inside_object, under the assumption that for finding the "caller" it is sufficient just to see if the address was between the start and end of the mappings. This is only possible if the mappings are right beside each other, because otherwise it's conceivable you might have two ELF files that actually interleave as long as their segment alignments allow it and the kernel assigns addresses to the gaps. > There is also a stray comment in the function itself. Fixed. >> Should we allow unit tests for the dynamic loader? > > I expect unit tests which look like this to be noncontroversial. Thank you for your input. > Things are more interesting if you have to add testing hooks to the > code, or export additional symbols. Agreed. My goal here was to be as open about my vision for testing. I think unit testing to define an interface is fine. I think unit testing to look for bugs is not fine. The unit testing should be simple enough to describe the way the interface works, but not contain a million regression tests. -- Cheers, Carlos.