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

Reply via email to