Re: [U-Boot] [PATCH v2 2/3] env: check and apply changes on delete/destroy
Dear Gerlando Falauto, In message <4ee60729.4020...@keymile.com> you wrote: > > > Run checkpatch, and mind the CodingStyle section about typedefs! > > I did run checkpatch, it didn't say a word about this. If you add new tyedef's, it will complain. > What is your recommendation about typedef'ining a function pointer? > I immediately suspected that using a callback function might sound like > heresy, so if you have any better suggestion please put it forward > before I rework this changeset for the fifth time... please. Sorry, I'm in the middle of -rc1 release work and testing, and also right in the final steps of releasing the next version of our ELDK (v5.1). Plus a few other tasks, including trips to customers. I don't have time for a thorough review now. I understand this is very bad for you, but I cannot change it. > I just think having the whole, expanded signature as the type for a > function-pointer parameter or local variable would just make things too > long and redundant. Hm... probbaly. > Just off the top of my head: perhaps a struct with a single function > pointer element might look better than a typedef? I'm not convinced. I'd have to see the code, and compare. But to do so, I'd need a bit of time... Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de A supercomputer is a machine that runs an endless loop in 2 seconds. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/3] env: check and apply changes on delete/destroy
On 12/12/2011 02:08 PM, Wolfgang Denk wrote: Dear Gerlando Falauto, In message<4ee5ca4a.8050...@keymile.com> you wrote: You could just use 'apply_cb apply' for that param I think. Absolutely. I introduced the typedef at a later stage and forgot to update it there too. Good catch, thanks! Run checkpatch, and mind the CodingStyle section about typedefs! I did run checkpatch, it didn't say a word about this. What is your recommendation about typedef'ining a function pointer? I immediately suspected that using a callback function might sound like heresy, so if you have any better suggestion please put it forward before I rework this changeset for the fifth time... please. I just think having the whole, expanded signature as the type for a function-pointer parameter or local variable would just make things too long and redundant. Just off the top of my head: perhaps a struct with a single function pointer element might look better than a typedef? Thanks, Gerlando Falauto ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/3] env: check and apply changes on delete/destroy
Dear Gerlando Falauto, In message <4ee5ca4a.8050...@keymile.com> you wrote: > > > You could just use 'apply_cb apply' for that param I think. > > Absolutely. I introduced the typedef at a later stage and forgot to > update it there too. Good catch, thanks! Run checkpatch, and mind the CodingStyle section about typedefs! Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de "I can call spirits from the vasty deep." "Why so can I, or so can any man; but will they come when you do call for them?" - Shakespeare, 1 King Henry IV, Act III, Scene I. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/3] env: check and apply changes on delete/destroy
On 12/07/2011 11:02 PM, Simon Glass wrote: diff --git a/include/search.h b/include/search.h index 2a59e03..7ad4261 100644 --- a/include/search.h +++ b/include/search.h @@ -142,7 +142,8 @@ int hcreate_r(size_t nel, struct hsearch_data *htab) * be freed and the local static variable can be marked as not used. */ -void hdestroy_r(struct hsearch_data *htab) +void hdestroy_r(struct hsearch_data *htab, + int(*apply)(const char *, const char *, const char *, int))\ You could just use 'apply_cb apply' for that param I think. Absolutely. I introduced the typedef at a later stage and forgot to update it there too. Good catch, thanks! Best, Gerlando ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v2 2/3] env: check and apply changes on delete/destroy
Hi Gerlando,
On Wed, Dec 7, 2011 at 5:30 AM, Gerlando Falauto
wrote:
> Signed-off-by: Gerlando Falauto
Tested-by: Simon Glass
> ---
> common/cmd_nvedit.c | 2 +-
> include/search.h | 6 --
> lib/hashtable.c | 18 --
> 3 files changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/include/search.h b/include/search.h
> index 2a59e03..7ad4261 100644
> --- a/include/search.h
> +++ b/include/search.h
> @@ -142,7 +142,8 @@ int hcreate_r(size_t nel, struct hsearch_data *htab)
> * be freed and the local static variable can be marked as not used.
> */
>
> -void hdestroy_r(struct hsearch_data *htab)
> +void hdestroy_r(struct hsearch_data *htab,
> + int(*apply)(const char *, const char *, const char *, int))\
You could just use 'apply_cb apply' for that param I think.
> {
> int i;
>
Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
