Re: [PATCH] Fix: transaction id usage in gisi/server.c

[Marcel Holtmann <marcel@holtmann.org>]

[-- Attachment #1: Type: text/plain, Size: 1668 bytes --]

Hi Remi,

> >> Casting to or from void is not necessary.
> > 
> > Oh, it is. If a struct sockaddr_pn structure is casted directly to
> > struct sockaddr, gcc will issue alignment warning on ARM.
> 
> The cast-align warning is a can of false positives. First that warning
> should probably not be enabled. It does nothing on x86, and it's a waste of
> time with ARM and other pointer-picky platforms.

I don't have an ARM platform easy available to test this, but this can
be fixed by just memcpy the data structures.

> Most importantly, using -Werror _by_default_ is a _damn_idiotic_ idea. Been
> there, done that. One cannot know what weird warning any given compiler or
> version thereof will ever return. And then I don't need to mention "bugs"
> in header files from underlying components. Even changing GCC optimization
> levels can yield different conflicting warnings. That's known to happen for
> uninitialized values detection, which relies on optimization-dependent code
> analysis as an example.

The rant about -Werror doesn't help. We are using it to catch nasty
constructs that might fail on any given platform. We have used -Werror
inside BlueZ and ConnMan for a long time now. Yes, sometimes new GCC
warnings come up and we fix that, but it was always good to have these.
And yes, it is true that you need to use -O2 to enable a bunch of these
warnings.

So in conclusion, the -Werror stays and we need to fix the code to
compile cleanly on any given platform. It is possible, so lets get this
done. Feel free to post the warnings on the mailing list and I am more
than happy to look into it.

Regards

Marcel