[U-Boot] [RFC, PATCH v4 01/16] hashtable: extend interfaces to handle entries with context

[Wolfgang Denk]

Dear AKASHI Takahiro,

In message <20190717082525.891-2-takahiro.akashi at linaro.org> you wrote:
> The following interfaces are extended to accept an additional argument,
> context:
>     hsearch_r() -> hsearch_ext()
>     hmatch_r() -> hmatch_ext()
>     hdelete_r() -> hdelete_ext()
>     hexport_r() -> hexport_ext()
>     himport_r() -> himport_ext()

Please avoid such renaming; keep the exiting names as is, and just
add new names (with descriptive names) where needed.

> --- a/include/search.h
> +++ b/include/search.h
> @@ -31,6 +31,7 @@ typedef enum {
>  } ACTION;
>  
>  typedef struct entry {
> +	unsigned int context;	/* opaque data for table operations */
>  	const char *key;

Please keep the key the first entry.


> @@ -230,6 +238,14 @@ static inline int _compare_and_overwrite_entry(ENTRY item, ACTION action,
>  {
>  	if (htab->table[idx].used == hval
>  	    && strcmp(item.key, htab->table[idx].entry.key) == 0) {
> +		/* No duplicated keys allowed */
> +		if (item.context != htab->table[idx].entry.context) {
> +			debug("context mismatch %s\n", item.key);
> +			__set_errno(EINVAL);
> +			*retval = NULL;
> +			return 0;
> +		}

If I understand correctly what you are doing here, then this needs a
different solution.  As is, the code just fails without a way to fix
this.  And the reason is that the matching is only done on the key,
while actually he context is important as well.  The logical
conclusion seems to be that the context must be part of the key.

> +int hsearch_r(ENTRY item, ACTION action, ENTRY ** retval,
> +	      struct hsearch_data *htab, int flag)
> +{
> +	ENTRY e;
> +
> +	e = item;
> +	e.context = 0;

Please do not use '0' here; use the proper enum name.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
In the beginning, I was made. I didn't ask to be made. No one consul-
ted with me or considered my feelings  in  this  matter.  But  if  it
brought  some  passing fancy to some lowly humans as they haphazardly
pranced their way through life's mournful jungle, then so be it.
- Marvin the Paranoid Android