On Sun, 04 Sep 2011 02:25:59 +0200 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
Am 03.09.2011 23:43 schrieb Stefan Tauner:
On Sat, 03 Sep 2011 14:46:00 +0200 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
Am 03.09.2011 12:33 schrieb Stefan Tauner:
On Fri, 02 Sep 2011 21:33:33 +0200 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote
Am 20.08.2011 12:39 schrieb Stefan Tauner:
because this patch relies on (compiler) implementation-specific layouting of bit-fields, it checks for basic compiler support in the makefile and for correct layout before taking any action on runtime.
Checking for presence of bitfield support in the compiler is the way to madness. Next we have to test for C99 capability and other fun stuff. Just assume that bitfields are supported, otherwise compilation will blow up or the runtime checks will catch a miscompilation. That compile-time check (including the #if BITFIELDS) is nacked.
well, for now bit-fields are only needed in one place that is seldom needed. why break compilation unnecessarily for all other cases too?
Would it really break compilation? AFAICS not.
it could due to: "A bit-field shall have a type that is a qualified or unqualified version of _Bool, signed int, unsigned int, or some other implementation-defined type."
maybe i don't understand the "qualified" part of this definition (i can not remember what this means in the specs right now), but i would think that uint32_t is not guaranteed to work as a bit-field type?
I thought that refers to the bitfield members, not the whole bitfield. We have to ask a C99 expert about that.
*shrug* can you please persuade michael to comment on this? :)
Bitfield support is available at least in all recent gcc and clang, and probably other compilers as well.
under the premise that all used compilers can deal with bit-fields the way we want, even the run time test is unnecessary ;)
You have to differentiate between the functionality mandated by C99 (I assume that as a given) and the functionality labeled as implementation defined by C99 (needs to be tested at runtime).
the question is if the implementation-defined functionality is two-fold and does not only create implementation-defined run time behavior but also have an impact on successful compiler termination. i was under the presumption that both can be a problem for us and handled them accordingly. there is no reason to discuss this approach further until we know if that premise is correct or false. :)
basically i agree, but since everyone thinks bit-fields break all the time i thought it is a good idea to integrate it in the least interfering way possible... :)
Bitfields have various implementation-defined aspects which may explode in your face unless you test for them at runtime, but it is technically impossible to test those at compile time.
if it would not possibly break compilation, i would totally agree with you. i am not insisting on keeping the test + ifdefs, but you have been warned now ;) please confirm what i should do.
If bitfields ever break compilation, we can handle the failure with some compiler dependent #ifdefs. Until then I'd rather avoid conditional code.
me too.
Could you comment on my "bitfield checker" mail? I know that endianness should not be complained about and rather be automatically fixed, but at least the code in there hopefully detects all possible bitfield layout variants allowed by C99.
i have skimmed through it before. what is the exact purpose? just checking if the implemented layout corresponds to our expectations (true/false)? the main problem i see with your test is that you are using the union only for read access and not write access, but write the data with memcpy only. imo writing via type pruning/unions should also be tested (i use that, but no memcpy iirc).
Semi-related question: Can we compile flashrom with gcc 2.95 (may break due to anonymous unions)? What about gcc 3.0/3.3?
not tested.
Can you start a separate discussion thread about error numbers? I see that Uwe has started assigning unique return codes to various ft2232_spi errors, and I was not really happy with those.
will do.
even then the whole descriptor is of interest imo... and this will most likely never occur in practice anyway.
A possible use case is allowing reflashing of a dev board with an additional jumper.
sure, but even then (or maybe then even more) the user would like to know the contents of the descriptor. the flag does indicate that the descriptor is valid only, nothing else. i don't see a problem.
Agreed. My comment was about the "likely never occur" aspect of your statement. We might want to handle FDOPSS by disregarding region protection registers, but I don't know if the documentation matches the hardware.
jup. also, the whole protection scheme handling in ichspi needs to be more refined (see the discussion in [PATCH 2/8] ichspi: disable writes when locked or read-only regions are detected). FDOPSS needs to be incorporated too... but this is unrelated to the hunk discussed here. the descriptor should always be printed if it seems to be valid (and the verbose level is sufficient of course).