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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to