Dear Joe Hershberger,
In message CANr=Z=b6wJCOxVc+zi6riiU4Smv5tv_5ifS1d9Ue8Ncv+xVLyg@mail.gmail.com you wrote:
I guess you didn't see this the last time I sent it to you off list...
I didn't receive any such reply, sorry. Do you have the message ID and/or exact timestanmp of your message?
- It appears you will store all access related information in the ACL variable (or a copy of it), i. e. in linearized form. Are you not concerned that this will make variable access slow?
At least in my uses so far, there are not so many variables that it is noticeably slower.
Did you run any benchmarks?
Note that in the linearized form not only the variable count, but also to total size of their values plays a role.
- There have been discussions that it would be nice to have a "volatile" variable type, for example for variables like "filesize" or "loadaddr" - such variables would not be stored to the persistent storage by the "saveenv" command. Thi sis something that should be fairly wasy to add to your code.
I was one of the proponents of this when we last discussed this some years ago. I left it out because you convinced me I should just use other variables and overwrite the special ones in a script when I need to. I still think supporting a volatile variable would be cleaner than that. I'm glad you changed your mind.
I always try to listen to what people tell me :-)
- Instead of parsing the ".flags" variable again and again when accessing variables, my idea was to extend the hash table: so far, the struct entry (see "include/search.h") holds only pointers to the key (variable name) and data (variable value). I suggest to extend this struct:
This seems like a good idea for the most part. Are you concerned that the hast table will take twice the space? Should this be optional?
The hash table lives in RAM only, which is usually an ample resource these days. I suggest to make the ".flags" support unconditional, but ".groups" support is probably not needed by many users, so this should be optional.
So the default configuration whould have only a 50% increase of the hash table size.
.callbacks=ethaddr:ethaddr,eth1addr,ath2addr;baudrate:baudrate;...One of the things I've run into with baudrate and silent specifically is wanting to control if the special callback is made on set and/or on env relocation, and/or on initial load, and/or on import. I think there should be a field in the flags for when to check the callback.
I don't see the need for this distinction, yet. My gut feeling is that the callback should be run when the varibale value in the hash table is being changed - relocation should play no role then, and initial import, "env set", "env import" (including variable deletion) should IMO all be handled the same.
I'd really like to make this code more simple.
This way we could even enale user specific callbacks without need to modify any common code.Would the user be able to define other callbacks somehow?
Yes. Similar as he can add "private" U-Boot commands in his board specific code, he should be able to add "private" callbacks.
I think is sounds very nice. And it's not too far from what I've done. I'll look into implementing some of it.
I was hoping for such a reply :-)
Thanks a lot in advance.
Best regards,
Wolfgang Denk