From mboxrd@z Thu Jan  1 00:00:00 1970
From: Gerlando Falauto <gerlando.falauto@keymile.com>
Date: Mon, 27 Aug 2012 09:45:38 +0200
Subject: [U-Boot] [PATCH v4 7/7] env: delete selected vars not present
	in imported env
In-Reply-To: <201208242312.09462.marek.vasut@gmail.com>
References: <1321634955-5561-1-git-send-email-gerlando.falauto@keymile.com>
	<201208241658.56669.marex@denx.de> <50379AD5.6030709@keymile.com>
	<201208242312.09462.marek.vasut@gmail.com>
Message-ID: <503B25A2.3070007@keymile.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 08/24/2012 11:12 PM, Marek Vasut wrote:
> Dear Gerlando Falauto,
>
>> On 08/24/2012 04:58 PM, Marek Vasut wrote:
>>> Dear Gerlando Falauto,
>>>
>>>> When variables explicitly specified on the command line are not present
>>>> in the imported env, delete them from the running env.
>>>> If the variable is also missing from the running env, issue a warning.
>>>>
>>>> Signed-off-by: Gerlando Falauto<gerlando.falauto@keymile.com>
>>>
>>> Whew! I made it through ... it wasn't that scary in the end ;-)
>>>
>>>> ---
>>>>
>>>>    lib/hashtable.c |   48
>>>>    +++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 41
>>>>    insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/lib/hashtable.c b/lib/hashtable.c
>>>> index f3f47de..b3d0b64 100644
>>>> --- a/lib/hashtable.c
>>>> +++ b/lib/hashtable.c
>>>> @@ -607,22 +607,32 @@ ssize_t hexport_r(struct hsearch_data *htab, const
>>>> char sep, * himport()
>>>>
>>>>     */
>>>>
>>>> -/* Check whether variable name is amongst vars[] */
>>>> -static int is_var_in_set(const char *name, int nvars, char * const
>>>> vars[]) +/*
>>>> + * Check whether variable 'name' is amongst vars[],
>>>> + * and remove all instances by setting the pointer to NULL
>>>> + */
>>>> +static int is_var_in_set(const char *name, int nvars, char * vars[])
>>>>
>>>>    {
>>>>
>>>>    	int i = 0;
>>>>
>>>> +	int res = 0;
>>>>
>>>>    	/* No variables specified means process all of them */
>>>>    	if (nvars == 0)
>>>>    	
>>>>    		return 1;
>>>>    	
>>>>    	for (i = 0; i<   nvars; i++) {
>>>>
>>>> -		if (!strcmp(name, vars[i]))
>>>> -			return 1;
>>>> +		if (vars[i] == NULL)
>>>> +			continue;
>>>> +		/* If we found it, delete all of them */
>>>> +		if (!strcmp(name, vars[i])) {
>>>> +			vars[i] = NULL;
>>>> +			res = 1;
>>>
>>> break here ?
>>
>> Nope, if we find it, we should delete all of them (see comment above).
>
> Stupid me, of course now I see the logic! Sorry!
>
>>>> +		}
>>>>
>>>>    	}
>>>>
>>>> -	debug("Skipping non-listed variable %s\n", name);
>>>> +	if (!res)
>>>> +		debug("Skipping non-listed variable %s\n", name);
>>>>
>>>> -	return 0;
>>>> +	return res;
>>>>
>>>>    }
>>>>
>>>>    /*
>>>>
>>>> @@ -662,9 +672,11 @@ static int is_var_in_set(const char *name, int
>>>> nvars, char * const vars[])
>>>>
>>>>    int himport_r(struct hsearch_data *htab,
>>>>
>>>>    		const char *env, size_t size, const char sep, int flag,
>>>>
>>>> -		int nvars, char * const vars[], int do_apply)
>>>> +		int nvars, char * const __vars[], int do_apply)
>>>
>>> Two underscores are reserved, use something else ;-)
>>
>> Like... one? three? ;-)
>
> I think one is the way to go ... http://lwn.net/Articles/509149/ definitelly not
> like this ;-)
>
> [...]

The way I understand it, two underscores are reserved for the compiler's 
internal use, whereas a single underscore is usually reserved for 
library function names.
So I'm sending a v5 of this patch (not the whole set) with some new 
naming altogether (is_var_in_set also deserves some better naming as the 
new implementation would otherwise make it very misleading).

Best regards,
Gerlando