From mboxrd@z Thu Jan  1 00:00:00 1970
From: Gerlando Falauto <gerlando.falauto@keymile.com>
Date: Fri, 30 Mar 2012 16:03:32 +0200
Subject: [U-Boot] [PATCH v2 1/3] env: unify logic to check and apply
	changes
In-Reply-To: <201203301555.00436.marex@denx.de>
References: <1321634955-5561-1-git-send-email-gerlando.falauto@keymile.com>
	<201203301508.37159.marex@denx.de> <4F75B38D.2070302@keymile.com>
	<201203301555.00436.marex@denx.de>
Message-ID: <4F75BD34.7050709@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 03/30/2012 03:55 PM, Marek Vasut wrote:

> Dear Gerlando Falauto,
>
>> On 03/30/2012 03:08 PM, Marek Vasut wrote:
>>> Dear Gerlando Falauto,
>>>
>>>> On 03/29/2012 10:19 PM, Marek Vasut wrote:
[...]
>>>>>> +		return 0;
>>>>>> +	}
>>>>>> +#endif
>>>>>> +	return 0;
>>>>>> +}
>>>>>> +
>>>>
>>>> [...]
>>>>
>>>>>> --- a/include/search.h
>>>>>> +++ b/include/search.h
>>>>>> @@ -47,6 +47,13 @@ typedef struct entry {
>>>>>>
>>>>>>     struct _ENTRY;
>>>>>>
>>>>>>     /*
>>>>>>
>>>>>> + * Callback function to be called for checking whether the given
>>>>>> change may + * be applied or not. Must return 0 for approval, 1 for
>>>>>> denial. + */
>>>>>> +typedef int (*apply_cb)(const char *name, const char *oldval,
>>>>>> +			const char *newval, int flag);
>>>>>
>>>>> Is the typedef really necessary ?
>>>>>
>>>>    >[From your other email]
>>>>    >
>>>>    >   I have to admit I'm not much of a fan of how you use this apply()
>>>>    >   callback, is it really necessary?
>>>>
>>>> Why ask, if you already know the answer? :-)
>>>>
>>>> I'm not a big fan either, seemed like the easiest approach at the time.
>>>> The idea was to keep the hastable (struct hsearch_data) as decoupled as
>>>> possible from the environment (env_htab which is, in fact, the only
>>>> instance of struct hsearch_data).
>>>>
>>>> What if the function pointer was stored within the hastable itself?
>>>> Sort of a virtual method.
>>>> This way we get rid of the typedef and the function pointer as a
>>>> parameter altogether.
>>>> The callback parameter then just becomes a boolean value (meaning,
>>>> do/don't call the callback function stored within the hashtable itself).
>>>> I like that much better. What do you think?
>>>
>>> Don't we always use only one (this callback) function?
>>
>> Yes, but only because env is the only hashtable present.
>> Is that a yes or a no, then?
>
> Do we expect any more hashtables in the near future ?

I don't think so. Anyway I would rather avoid calling functions 
belonging to the environment domain from the hastable domain directly.
For that matter, we have a single "struct hsearch_data" instance in the 
whole project, but we keep passing it around as an argument to the 
hashtable functions.

Best,
Gerlando