On 12/10/2014 08:32 AM, Bill Fischofer wrote:
Just as an FYI, I took a stab at classifying the issues with std=c99.
The root issue is that c99 is more than just a C language issue--it
also implies that you're restricted to using the standard C library
,and most of the examples (and odp_shared_memory.c) violate this
restriction.
I think for examples there is no need to be C99. Examples is only odp
library usage. So it can be even C++, python. I think.
Maxim.
The "cheat" is to include the line:
#define _BSD_SOURCE
which "unhides" the non-standard library functions. But the real
answer is that to be strictly c99 compliant you need to go back and
replace the non-standard library calls with their standard
equivalents. Unfortunately many of these do not have standard
equivalents, or at least not strict equivalents, so this may involve
redesigning the code.
For the examples, the cheat is probably OK since they are illustrating
application use of ODP and we never said that ODP /applications/ need
to be c99 compliant. However odp_shared_memory.c /is/ part of ODP and
hence should be held to that higher standard. In this routine's case
the offending routine is ftruncate(). I'm not a C library guru so I'm
not sure how one would replace that call with something that passes
muster, but that's the key sticking point.
The existing odp_timer.c routine has similar issues, but since that's
being replaced with Ola's new timer code I assume that shouldn't be a
problem.
Moral: If you learned to program C applications prior to C99, you
probably aren't writing C99-compliant code because you're naturally
reaching for non-standard library routines without thinking about this
problem.
Bill
On Tue, Dec 9, 2014 at 1:46 PM, Bill Fischofer
<[email protected] <mailto:[email protected]>> wrote:
Thanks! I'll fix that in the v3 patch as well.
Bill
On Tue, Dec 9, 2014 at 1:22 PM, Stuart Haslam
<[email protected] <mailto:[email protected]>> wrote:
On Tue, Dec 09, 2014 at 06:20:02PM +0000, Bill Fischofer wrote:
> I'll take a look at odp_shared_memory. Maxim/Stuart should
take a look at odp_packet_io.c. I assume Ola's new timer code
is compliant. If not, it should be.
>
> Bill
I just sent a patch that clears the caddr_t failure.
Note that I also get build failures on one of my dev branches
which has
the buffer pool patch applied, due to the use of typeof.
--
Stuart.
>
> On Tue, Dec 9, 2014 at 12:10 PM, Mike Holmes
<[email protected]
<mailto:[email protected]><mailto:[email protected]
<mailto:[email protected]>>> wrote:
>
>
> On 9 December 2014 at 12:57, Bill Fischofer
<[email protected]
<mailto:[email protected]><mailto:[email protected]
<mailto:[email protected]>>> wrote:
> If we can get a complete list of which files currently have
issues I'm thinking that would be an excellent list to mull
over during the Christmas break.
>
> For a list just add the cflags line "./configure
CFLAGS=-std=c99" and then make -k 2>&1 | grep "^.*\.c:"
>
> Api is currently
>
> odp_packet_io.c
> odp_shared_memory.c
> odp_timer.c
>
> There are more in the examples
>
> For those of us who will be "off" we may still want a coding
fix for an hour or so. :)
>
> Also it's an excellent way for those on the ODP mailing list
who want to "get their hands dirty" with ODP code to tackle
something small and focused.
>
> Bill
>
> On Tue, Dec 9, 2014 at 11:54 AM, Mike Holmes <[email protected]
<mailto:[email protected]><mailto:[email protected]
<mailto:[email protected]>>> wrote:
> I think adding it explicitly when using the patch checking
tool "apply-and-build.sh" might be a way forward.
>
> If we can get agreement on C99, then bugzilla already has
many of these bugs
listed,<https://bugs.linaro.org/buglist.cgi?component=General&list_id=3080&product=OpenDataPlane&resolution=--->
including the source of Robbies issue. All we need is
agreement that C99 is our direction and git blame will show us
the likely best person to fix each issue.
>
> On 9 December 2014 at 12:48, Bill Fischofer
<[email protected]
<mailto:[email protected]><mailto:[email protected]
<mailto:[email protected]>>> wrote:
> OK, I understand this is perhaps a longer-term project, but
is there a file-level way we can enable this check? If yes,
then we can ask that as part of modules that are currently
being patched that c99 checks be included in them. That way
files will be brought into compliance and then stay in
compliance after they've been merged.
>
> On Tue, Dec 9, 2014 at 11:40 AM, Mike Holmes <[email protected]
<mailto:[email protected]><mailto:[email protected]
<mailto:[email protected]>>> wrote:
> sorry, I cleaned the history try this:
>
https://ci.linaro.org/view/odp-ci/job/odp-api-check-native-c99/buildhw=x86_64,label=build/16/console
>
> On 9 December 2014 at 12:39, Mike Holmes <[email protected]
<mailto:[email protected]><mailto:[email protected]
<mailto:[email protected]>>> wrote:
> Make -k gives the attached so it is easy to look past the
first issue.
> We cant make it default because everything breaks, we should
fix things and then the CI job which
> has been building it this way for a while will eventually
pass. If we can get consensus on linux-generic sticking to to
C99 it is worth following up on this build.
>
> The builds are for both ARM and X86, I had not been
following it becasue we did not have consensus on the C99
issue: X86 is working but Arm filesystem needs curl installing.
>
>
https://ci.linaro.org/view/odp-ci/job/odp-api-check-native-c99/buildhw=x86_64,label=build/14/console
>
> We dont want to hijack the need to have ./configure check
for the version we are using however.
>
>
> On 9 December 2014 at 12:17, Bill Fischofer <[email protected]
<mailto:[email protected]><mailto:[email protected]
<mailto:[email protected]>>> wrote:
> I suggest we turn on that by default. It's the one way to
ensure that the issues will get fixed. Is this a
stop-on-first-error situation or can we have it carry on so we
get a complete list of what the c99 issues are?
>
> On Tue, Dec 9, 2014 at 11:02 AM, Mike Holmes <[email protected]
<mailto:[email protected]><mailto:[email protected]
<mailto:[email protected]>>> wrote:
> This might also be the right time to adhere to the use of
only C99 in linux-generics implementation ?
>
> CFLAGS=-std=c99 ./configure
> make
>
> Throws up other issues, the first is in packet_io
>
> mike@fedora1:~/git/odp$ make
> Making all in platform
> make[1]: Entering directory '/home/mike/git/odp/platform'
> Making all in linux-generic
> make[2]: Entering directory
'/home/mike/git/odp/platform/linux-generic'
> CC odp_packet_io.lo
> odp_packet_io.c: In function 'odp_pktio_set_mtu':
> odp_packet_io.c:512:35: error: 'caddr_t' undeclared (first
use in this function)
> ret = ioctl(sockfd, SIOCSIFMTU, (caddr_t)&ifr);
> ^
> odp_packet_io.c:512:35: note: each undeclared identifier is
reported only once for each function it appears in
> Makefile:560: recipe for target 'odp_packet_io.lo' failed
>
>
>
> Mike
>
> On 9 December 2014 at 11:48, Robbie King (robking) <[email protected]
<mailto:[email protected]><mailto:[email protected]
<mailto:[email protected]>>> wrote:
> After cloning tip just now, I found that I couldn’t build
the fresh
> workspace due to having GCC 4.6.3 (the C11 changes to the
atomics
> bumps minimum GCC up to 4.8 as best I can tell). I’m not
very familiar
> with what “./configure” can and can’t do, but it seems we
should verify
> the compiler supports these constructs and fail during the
configure
> phase (as opposed to build time).
>
> Thanks,
> Robbie
>
> _______________________________________________
> lng-odp mailing list
> [email protected]
<mailto:[email protected]><mailto:[email protected]
<mailto:[email protected]>>
>http://lists.linaro.org/mailman/listinfo/lng-odp
>
>
>
>
> --
> Mike Holmes
> Linaro Sr Technical Manager
> LNG - ODP
>
> _______________________________________________
> lng-odp mailing list
> [email protected]
<mailto:[email protected]><mailto:[email protected]
<mailto:[email protected]>>
>http://lists.linaro.org/mailman/listinfo/lng-odp
>
>
>
>
>
> --
> Mike Holmes
> Linaro Sr Technical Manager
> LNG - ODP
>
>
>
> --
> Mike Holmes
> Linaro Sr Technical Manager
> LNG - ODP
>
>
>
>
> --
> Mike Holmes
> Linaro Sr Technical Manager
> LNG - ODP
>
>
>
>
> --
> Mike Holmes
> Linaro Sr Technical Manager
> LNG - ODP
>
> _______________________________________________
> lng-odp mailing list
> [email protected] <mailto:[email protected]>
> http://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp
