Dear Guennadi Liakhovetski,
In message Pine.LNX.4.64.0809020104080.8933@axis700.grange you wrote:
As you know, in the tool we have to decide at run-time whether we are dealing with a single environment copy or with current / redundant configuration. With NAND support when _writing_ environment to NAND you have to write page at a time, which means, at this point we _must_ have the image in a contiguous buffer in RAM. And the image can have one of the
Agreed so far.
two possible formats - with and without the "flags" byte. In principle I see only three possibilities to implement this:
(a) work with arbitrary non-contiguous data in RAM as before, and copy it into an additional buffer just before writing to NAND
advantage: can keep the current struct
disadvantage: extra malloc
Agreed.
(b) use a plain data buffer, and, if needed, use the first byte in it for flags
advantage: no extra malloc, no (explicit) union
disadvantage: confusing, have to work with byte-offsets instead of struct / union members, and, in fact, this is the same as using a union, just implicit, calculating byte-offsets manually, instead of letting the compiler do it
Agreed that this is even worse than a union.
(c) use a union
advantage: clean access to all environment fields without the use of byte-offsets
disadvantage: slightly more complex code
Please, just tell me which of these three you would prefer, or maybe there
None of them, I think.
is a fourth possibility I am still overseeing.
The two cases (redundant versus non-redundant env) are well separated, and known early (after parsing the config file, i. e. before any processing of environment data).
How about defining two structs, one without the flag byte (non- redundant env), and another one with the flag byte included (redundant env). Then just use a pointer of the correct type (either first or second struct) to access the data.
You also asked about the extra "char *data" pointer in the struct environment, whether there is no danger that a different compiler version will break it. This pointer uses no magic - it is just a plain simple pointer, I use it to point to data inside the union to avoid having to check every time whether we have the redundant environment or not. So, I check it only once at initialisation time, set this pointer, and then just use it to access the data buffer inside the image (union). No magic here.
But it's bogus. Now you have data[] in the union, *plus* in the struct. You have it twice.
- fix MTD_OLD
Would we still need this with NAND-only tool?
Yes of course we need it. I will not accept such thing as a NAND-only tool.
Ok. But NAND-support is not needed with MTD_OLD? So, if it cannot be compiled with older kernels, we may just disable it per ifdef?
Do we really need to ifdef this?
How would you like to make such a replacement then? If I produce a patch just from the current state to the final state, I think, it will look worse than the broken-down patch-series. Otherwise we could remove the
It will not. See my previous statistics. It will be less than half the size of your split patches.
Best regards,
Wolfgang Denk