On 12/07/2011 11:02 PM, Simon Glass wrote:
[...]
/*
- * Set a new environment variable,
- * or replace or delete an existing one.
+ * Performs consistency checking before setting, replacing,
+ * or deleting an environment variable, then (if successful)
+ * apply the changes to internals so to make them effective.
+ * Code for this function was taken out of _do_env_set(),
+ * which now calls it.
+ * Also called as a callback function by himport_r().
+ * Returns 0 in case of success, 1 in case of failure.
+ * When (flag& H_FORCE) is set, force overwriting of
+ * write-once variables.
can you word-wrap that to 72 columns perhaps?
Sorry, I am little confused now. What is the maximum line length?
[...]
+/* Check whether variable name is amongst vars[] */
+static int process_var(const char *name, int nvars, char * const vars[])
+{
+ int i = 0;
blank line here.
Thanks, I didn't know about this.
Can part of this function be #ifdefed to reduce code
size when the feature is not required?
Uhm, I don't think so. This would be a common feature for selectively
importing & setting back to default. Unless we want to #ifdef both of
them. Personally, I would #ifdef neither.
[...]
+ if (!process_var(name, nvars, vars))
+ continue;
if (hdelete_r(name, htab) == 0)
debug("DELETE ERROR
##############################\n");
perhaps:
if (process_var(name, nvars, vars)&&
hdelete_r(name, htab) == 0)
debug("DELETE ERROR ##############################\n");
I think it's easier to read it the original way, and it should not make
any difference as far as code size is concerned.
himport_r() is getting a bit overloaded,
Actually, I believe it makes no longer sense to have it called "_r", as
it was the original reference to the function being recursively
calleable (i.e. reentrant) as opposed to other versions which were not.
and it's a shame but I can't
think of an easy way to refactor it to reduce the number of args. In a
way you are adding a processing option and so you could put the three
extra args in a structure and pass a pointer to it (or NULL to skip
this feature). Not sure though...
Can't think of any other way either, except maybe renaming it and
re-implementing the shorter version as a wrapper around the newly named
function. I had already done that, but there would be very few places
where the old syntax would be kept, so it just didn't make much sense.
Also, for me this patch adds 500 bytes. I wonder if more of the code
could made optional?
Frankly, I'm surprised to hear this adds that much overhead.
Actually, I can't see this increase in code size as you mention.
What architecture are you referring to?
In my workspace (ppc_6xx) u-boot.bin and a stripped u-boot ELF file are
surprisingly unchanged in size, even with debug #defined!
Only place I can experience such growth is within unstripped u-boot ELF,
which I believe shouldn't really matter... or should it?
Best,
Gerlando
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
