From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 90025 invoked by alias); 19 Jun 2017 16:38:11 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 89945 invoked by uid 89); 19 Jun 2017 16:38:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=guiding, Hx-languages-length:1697 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 19 Jun 2017 16:38:10 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C2AD885365; Mon, 19 Jun 2017 16:38:13 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com C2AD885365 Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=palves@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com C2AD885365 Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5D95618501; Mon, 19 Jun 2017 16:38:11 +0000 (UTC) Subject: Re: [PATCH v6] C++ify gdb/common/environ.c To: Sergio Durigan Junior References: <20170413040455.23996-1-sergiodj@redhat.com> <20170619043531.32394-1-sergiodj@redhat.com> <87bmpkx8fb.fsf@redhat.com> Cc: GDB Patches , Simon Marchi From: Pedro Alves Message-ID: <566ca0fa-e9ed-bc57-82d1-3152acc3dab2@redhat.com> Date: Mon, 19 Jun 2017 16:38:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <87bmpkx8fb.fsf@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-06/txt/msg00527.txt.bz2 On 06/19/2017 05:13 PM, Sergio Durigan Junior wrote: > On Monday, June 19 2017, Pedro Alves wrote: >>> + >>> + gdb_environ env; >>> + >>> + SELF_CHECK (env.envp ()[0] == NULL); >>> + >>> + SELF_CHECK (env.get ("PWD") == NULL); >>> + env.set ("PWD", "test"); >>> + env.unset ("PWD"); >>> + >> >> Please add another >> >> SELF_CHECK (env.envp ()[0] == NULL); >> >> after the unset. I didn't spot any check making sure >> that invariant holds after an unset. > > This invariant is not supposed to hold after every unset, only after a > clear or after an unset that removes the only variable in the vector. ... which is exactly the case above. And for unsets where there are still elements, the invariant is that the last element is NULL [which is of course the same invariant]. So maybe we should have a little function like this (could reuse countargv too): static size_t countenvp (const gdb_environ &env) { char **envp = env.envp (); size_t count = 0; while (envp[count] != NULL) count++; return count; } Used instead of the NULL SELF_CHECKs, like: gdb_environ env; /* This makes sure that env.envp() is NULL terminated. */ SELF_CHECK (countenvp (env) == 0); /* ENV is empty, so we shouldn't be able to find any var. */ SELF_CHECK (env.get ("PWD") == NULL); /* Set a var, and make sure that env.envp() is still NULL terminated. */ env.set ("PWD", "test"); SELF_CHECK (countenvp (env) == 1); /* Clear the var and make sure that env.envp() is left NULL terminated when we remove the last element. */ env.unset ("PWD"); SELF_CHECK (countenvp (env) == 0); etc. I find that adding guiding comments like above helps, btw. Thanks, Pedro Alves