On Sat, Sep 19, 2015 at 11:32:54AM +0200, Bartosz Golaszewski wrote: > 19 wrz 2015 12:25 AM "Aurelien Jacobs" <au...@gnuage.org> napisaĆ(a): > > > > On Fri, Sep 18, 2015 at 08:09:53PM +0200, Uwe Hermann wrote: > > > Hi, > > > > > > On Thu, Sep 17, 2015 at 11:59:42AM +0200, Bartosz Golaszewski wrote: > > > > http://git.baylibre.com/pub/acme/libsigrok/ revB > > > > > > Merged, thanks! > > > > > > Looks good, one minor improvement you could do is use g_ntohl() instead > > > of ntohl(), which also avoids the inet.h include. > > > > Or maybe you should not use ntohl() at all. > > But more importantly, you can't map the eeprom content directly > > to a struct whose size will vary depending on the target arch, and you > > probably shouldn't rely on a packed struct which is not standard C (even > > though it is probably supported by all sigrok relevant compilers). > > Hi Aurelien, > > this is a very stupid bug using int and unsigned long from my side. > Especially when I actually use uint32 etc. in the program creating the > EEPROM contents. > > Tell me if I'm wrong but simply changing these to appropriate fixed-size > types should be enough in this case right?
This might be working with compilers properly supporting __attribute__((packed)) But this is not part of C99 or any other standard, so you would have to check for its support in the configure script and disable the baylibre-acme driver for compilers that do not support it. It would still be much simpler, cleaner and more portable to just use standard C99 code for this simple task, and more consistant with other drivers. Many drivers in sigrok are already dealing with incoming binary stream with fixed fields size and endianness, and none of them are mapping those data to a packed struct... For a reason. Aurel ------------------------------------------------------------------------------ _______________________________________________ sigrok-devel mailing list sigrok-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/sigrok-devel