Bug 47191 - Missing sentinel warning
Summary: Missing sentinel warning
Status: NEW
Alias: None
Product: APR
Classification: Unclassified
Component: APR (show other bugs)
Version: HEAD
Hardware: All All
: P2 minor (vote)
Target Milestone: ---
Assignee: Apache Portable Runtime bugs mailinglist
URL:
Keywords: PatchAvailable
Depends on:
Blocks:
 
Reported: 2009-05-13 03:39 UTC by Rainer Jung
Modified: 2009-05-14 03:24 UTC (History)
0 users



Attachments
Adding stddef.h to header files using sentinel attribute. (1.91 KB, patch)
2009-05-13 03:39 UTC, Rainer Jung
Details | Diff
Minimal patch, including stddef.h unconditionally in apr.h (879 bytes, patch)
2009-05-14 03:22 UTC, Rainer Jung
Details | Diff
The mid-size patch: include in apr.h, but detect via configure. (1.48 KB, patch)
2009-05-14 03:24 UTC, Rainer Jung
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rainer Jung 2009-05-13 03:39:42 UTC
Created attachment 23650 [details]
Adding stddef.h to header files using sentinel attribute.

In revision 683278 a sentinel attribute mark was introduced to some APR
functions (trunk and 1.4).

On Solaris 8 and 10 using GCC 4 I get warnings about missing sentinels in function calls. It turns out, that this is due to "NULL" being defined as "0" instead of e.g. (void *)0.

This comes from the inclusion of sys/types.h, which leads to iso/time_iso.h being included. That file defines NULL as 0.

sys/types.h occurs very early in the list of includes in apr.h

For gcc 4.1.2 I can add stddef.h, because the fixincludes
version of it defines NULL as (void *)0.

Unfortunately apr does set HAVE_STDDEF_H only in apr_private.h, so we can't simply add something like

#ifdef HAVE_STDDEF_H
#include <stddef.h> /* NULL */
#endif

to apr_strings.h etc. I think we need to add APR_HAVE_STDDEF_H to
apr.h(.in) (and configure) so we can use

#if APR_HAVE_STDDEF_H
#include <stddef.h> /* NULL */
#endif

in apr_strings.h and apr_tables.h.

I attach a patch for trunk and 1.4.x. The sentinel attribute does not exist in other branches.

Regards,

Rainer
Comment 1 Joe Orton 2009-05-13 06:17:31 UTC
All supported platforms should have stddef.h, just include it unconditionally.
Comment 2 Rainer Jung 2009-05-13 06:28:12 UTC
Do you suggest to include it directly in apr.h (like we do e.g. in apr.hw), or like my patch does more fine-grained in those header files, that actually declare a function with a sentinel attribute?
Comment 3 Joe Orton 2009-05-13 06:53:36 UTC
Doing it in apr.h near the includes for sys/types.h et al seems fine to me.
Comment 4 Rainer Jung 2009-05-14 03:22:27 UTC
Created attachment 23661 [details]
Minimal patch, including stddef.h unconditionally in apr.h

This is the minimal patch, I guess the way as proposed by Joe.
Comment 5 Rainer Jung 2009-05-14 03:24:27 UTC
Created attachment 23662 [details]
The mid-size patch: include in apr.h, but detect via configure.

This is the variant I propose, because it includes stddef.h in apr.h, so we don't need to put it into every future header file where the attribute mark gets used, but on the other hand it is more consistent with the rest of apr.h (detecting the header file).