[PATCH] string.h, _BSD_SOURCE, and *index()

[PATCH] string.h, _BSD_SOURCE, and *index()

[Isaac Dunham]

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

I was working on _BSD_SOURCE for string.h.
Warning: several of the functions are mislabeled as _GNU_SOURCE in the
manpages. 
Everything in strings.h should be available if _GNU_SOURCE ||
BSD_SOURCE is defined and  string.h is included.
Actually, that's slightly an oversimplification-there are two
functions (*_l) that should be _GNU_SOURCE only.

(r)index was X/Open legacy, and has been dropped. The Open Group
recommended using 
#define index(a,b) strchr((a),(b))
#define rindex(a,b) strrchr((a),(b))
Which will let us remove two more files if we do it (rindex.c & index.c)
However, would removing those break the ABI?

There are two patches attached: the first (string.diff) will define
everything from strings.h if defined _BSD_SOURCE || _GNU_SOURCE
The second patch (rindex.diff) applies the second change, but does not
remove (r)index.c, to prevent ABI breakage.

Isaac Dunham

[-- Attachment #2: string.diff --]
[-- Type: text/x-patch, Size: 2056 bytes --]

diff --git a/include/string.h b/include/string.h
index 4aa930e..cd09513 100644
--- a/include/string.h
+++ b/include/string.h
@@ -14,7 +14,7 @@ extern "C" {
 
 #define __NEED_size_t
 #if defined(_POSIX_SOURCE) || defined(_POSIX_C_SOURCE) \
- || defined(_XOPEN_SOURCE) || defined(_GNU_SOURCE)
+ || defined(_XOPEN_SOURCE) || defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
 #define __NEED_locale_t
 #endif
 
@@ -53,7 +53,7 @@ char *strerror (int);
 
 
 #if defined(_POSIX_SOURCE) || defined(_POSIX_C_SOURCE) \
- || defined(_XOPEN_SOURCE) || defined(_GNU_SOURCE)
+ || defined(_XOPEN_SOURCE) || defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
 char *strtok_r (char *, const char *, char **);
 int strerror_r (int, char *, size_t);
 char *stpcpy(char *, const char *);
@@ -67,10 +67,21 @@ int strcoll_l (const char *, const char *, locale_t);
 size_t strxfrm_l (char *, const char *, size_t, locale_t);
 #endif
 
-#if defined(_XOPEN_SOURCE) || defined(_GNU_SOURCE)
+#if defined(_XOPEN_SOURCE) || defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
 void *memccpy (void *, const void *, int, size_t);
 #endif
 
+#if defined(_BSD_SOURCE) || defined(_GNU_SOURCE)
+int bcmp (const void *, const void *, size_t);
+void bcopy (const void *, void *, size_t);
+void bzero (void *, size_t);
+int strcasecmp (const char *, const char *);
+int strncasecmp (const char *, const char *, size_t);
+char *index (const char *, int);
+char *rindex (const char *, int);
+int ffs (int);
+#endif
+
 #ifdef _BSD_SOURCE
 size_t strlcat (char *, const char *, size_t);
 size_t strlcpy (char *, const char *, size_t);
@@ -78,8 +89,8 @@ size_t strlcpy (char *, const char *, size_t);
 
 #ifdef _GNU_SOURCE
 int strverscmp (const char *, const char *);
-int strcasecmp (const char *, const char *);
-int strncasecmp (const char *, const char *, size_t);
+int strcasecmp_l (const char *, const char *, locale_t);
+int strncasecmp_l (const char *, const char *, size_t, locale_t);
 char *strchrnul(const char *, int);
 char *strcasestr(const char *, const char *);
 char *strsep(char **, const char *);

[-- Attachment #3: index.diff --]
[-- Type: text/x-patch, Size: 928 bytes --]

diff --git a/include/string.h b/include/string.h
index cd09513..ab9ba2d 100644
--- a/include/string.h
+++ b/include/string.h
@@ -77,8 +77,8 @@ void bcopy (const void *, void *, size_t);
 void bzero (void *, size_t);
 int strcasecmp (const char *, const char *);
 int strncasecmp (const char *, const char *, size_t);
-char *index (const char *, int);
-char *rindex (const char *, int);
+#define index(a,b) strchr((a),(b))
+#define rindex(a,b) strrchr((a),(b))
 int ffs (int);
 #endif
 
diff --git a/include/strings.h b/include/strings.h
index 345c651..f8fd255 100644
--- a/include/strings.h
+++ b/include/strings.h
@@ -17,8 +17,8 @@ void bzero (void *, size_t);
 
 int ffs (int);
 
-char *index (const char *, int);
-char *rindex (const char *, int);
+#define index(a,b) strchr((a),(b))
+#define rindex(a,b) strrchr((a),(b))
 
 int strcasecmp (const char *, const char *);
 int strncasecmp (const char *, const char *, size_t);

Re: [PATCH] string.h, _BSD_SOURCE, and *index()

[Andre Renaud]

On Fri, Apr 13, 2012 at 1:45 PM, Isaac Dunham <idunham@lavabit.com> wrote:
> (r)index was X/Open legacy, and has been dropped. The Open Group
> recommended using
> #define index(a,b) strchr((a),(b))
> #define rindex(a,b) strrchr((a),(b))
> Which will let us remove two more files if we do it (rindex.c & index.c)
> However, would removing those break the ABI?

I'm curious about this - what is the general consensus on using
#define to do these kind of translations, versus using static inline
functions, such as:
static inline char *index(const char *s, int c) {return strchr(s, c);}
static inline char *rindex(const char *s, int c) {return strrchr(s, c);}

Regards,
Andre

Re: [PATCH] string.h, _BSD_SOURCE, and *index()

[Rich Felker]

On Fri, Apr 13, 2012 at 01:52:27PM +1200, Andre Renaud wrote:
> On Fri, Apr 13, 2012 at 1:45 PM, Isaac Dunham <idunham@lavabit.com> wrote:
> > (r)index was X/Open legacy, and has been dropped. The Open Group
> > recommended using
> > #define index(a,b) strchr((a),(b))
> > #define rindex(a,b) strrchr((a),(b))
> > Which will let us remove two more files if we do it (rindex.c & index.c)
> > However, would removing those break the ABI?
> 
> I'm curious about this - what is the general consensus on using
> #define to do these kind of translations, versus using static inline
> functions, such as:
> static inline char *index(const char *s, int c) {return strchr(s, c);}
> static inline char *rindex(const char *s, int c) {return strrchr(s, c);}

Both are wrong to do in this header, but the static inline definition
is even "more wrong". Non-static inline would be okay, but static
inline conflicts with the "correct" (albeit removed) declaration of
the function and will actively break code that prototypes the
function (and can't be removed with #undef).

Rich

Re: [PATCH] string.h, _BSD_SOURCE, and *index()

[Rich Felker]

On Thu, Apr 12, 2012 at 06:45:22PM -0700, Isaac Dunham wrote:
> I was working on _BSD_SOURCE for string.h.
> Warning: several of the functions are mislabeled as _GNU_SOURCE in the
> manpages. 
> Everything in strings.h should be available if _GNU_SOURCE ||
> BSD_SOURCE is defined and  string.h is included.

#if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
#include <strings.h>
#endif

Actually I thought that was already there (minus the BSD) but it seems
I was mistaken. In any case, I think it's cleaner than duplicating all
the prototypes in 2 places.

> Actually, that's slightly an oversimplification-there are two
> functions (*_l) that should be _GNU_SOURCE only.

OK.

> (r)index was X/Open legacy, and has been dropped. The Open Group
> recommended using 
> #define index(a,b) strchr((a),(b))
> #define rindex(a,b) strrchr((a),(b))
> Which will let us remove two more files if we do it (rindex.c & index.c)
> However, would removing those break the ABI?

Yes. It would also break the API for programs which declare these
functions manually rather than using the header (that's more popular
than you think), and as your patch is written it's violating the
SUSv4 namespace (since these symbols were removed, you can't define
them). The advice on using macros is advice for _applications_, not
for implementations.

The proper fix is just to move the declarations inside
_GNU_SOURCE/_BSD_SOURCE.

Rich

Re: [PATCH] string.h, _BSD_SOURCE, and *index()

[Isaac Dunham]

On Thu, 12 Apr 2012 23:30:13 -0400
Rich Felker <dalias@aerifal.cx> wrote:

> On Thu, Apr 12, 2012 at 06:45:22PM -0700, Isaac Dunham wrote:
> > I was working on _BSD_SOURCE for string.h.
> > Warning: several of the functions are mislabeled as _GNU_SOURCE in
> > the manpages. 
> > Everything in strings.h should be available if _GNU_SOURCE ||
> > BSD_SOURCE is defined and  string.h is included.
> 
> #if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
> #include <strings.h>
> #endif
> 
> Actually I thought that was already there (minus the BSD) but it seems
> I was mistaken. In any case, I think it's cleaner than duplicating all
> the prototypes in 2 places.
Definitely cleaner...
but there are a few repeat definitions. (Some of it is now
_POSIX_SOURCE).  But that probably won't break things.
And it could slow down compiles, etc.
> 
> > (r)index was X/Open legacy, and has been dropped. The Open Group
> > recommended using 
> > #define index(a,b) strchr((a),(b))
> > #define rindex(a,b) strrchr((a),(b))
> > Which will let us remove two more files if we do it (rindex.c &
> > index.c) However, would removing those break the ABI?
> 
> Yes. It would also break the API for programs which declare these
> functions manually rather than using the header (that's more popular
> than you think), and as your patch is written it's violating the
> SUSv4 namespace (since these symbols were removed, you can't define
> them). 

Huh?

|+#if defined(_BSD_SOURCE) || defined(_GNU_SOURCE)
|+int bcmp (const void *, const void *, size_t);
|+void bcopy (const void *, void *, size_t);
|+void bzero (void *, size_t);
|+int strcasecmp (const char *, const char *);
|+int strncasecmp (const char *, const char *, size_t);
|+char *index (const char *, int);
|+char *rindex (const char *, int);
|+int ffs (int);
|+#endif

I didn't move them in the second patch, either.

>The advice on using macros is advice for _applications_, not
> for implementations.
OK. That answers the question.  I wasn't sure, so I did two patches.