This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v6] C++ify gdb/common/environ.c
- From: Pedro Alves <palves at redhat dot com>
- To: Sergio Durigan Junior <sergiodj at redhat dot com>
- Cc: GDB Patches <gdb-patches at sourceware dot org>, Simon Marchi <simon dot marchi at polymtl dot ca>
- Date: Mon, 19 Jun 2017 17:38:10 +0100
- Subject: Re: [PATCH v6] C++ify gdb/common/environ.c
- Authentication-results: sourceware.org; auth=none
- 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 at redhat dot com
- Dkim-filter: OpenDKIM Filter v2.11.0 mx1.redhat.com C2AD885365
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com C2AD885365
- References: <20170413040455.23996-1-sergiodj@redhat.com> <20170619043531.32394-1-sergiodj@redhat.com> <c13db0c7-ad1f-5ba6-ff95-12004fba9904@redhat.com> <87bmpkx8fb.fsf@redhat.com>
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