On Fri, Jul 19, 2019 at 08:58:56AM +0200, Wolfgang Denk wrote:
Dear AKASHI Takahiro,
In message 20190717082525.891-2-takahiro.akashi@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.
As I said in a reply to your previous comment, I have not renamed them.
--- 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.
Why?
@@ -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 intention here is to prevent a context of any entry from being changed. I believe that this is a reasonable assumption and restriction.
The logical conclusion seems to be that the context must be part of the key.
Do you mean that an actual name of variable, for example, "foo" should be, say, "uboot_foo"?
This will also make the implementation of env_save/load a bit ugly again.
+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.
I intentionally did so because I don't want to pollute "hash table" functions with env-specific features so that hash functions will be used for other purposes. I admit that it is already polluted with flags though.
-Takahiro Akashi
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@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