[PATCH] Mac OS X compatibility fixes

[PATCH] Mac OS X compatibility fixes

[Jjgod Jiang]

Add missing GNU extensions strdup() and getline(). The C library
shipped with Mac OS X does not include them (though it does support
some GNU extensions when _GNU_SOURCE is defined), so we have to
add these two. The getline() implementation is a modified version
of getdelim() from GNU Libc.
---
 lib/xutil.c |   99 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/xutil.h |    6 +++
 2 files changed, 105 insertions(+), 0 deletions(-)

diff --git a/lib/xutil.c b/lib/xutil.c
index 6fa5eb0..805b236 100644
--- a/lib/xutil.c
+++ b/lib/xutil.c
@@ -79,6 +79,105 @@ xstrdup (const char *s)
     return ret;
 }
 
+#ifdef __APPLE__
+/* Mac OS X don't have strndup even if _GNU_SOURCE is defined */
+char *strndup (const char *s, size_t n)
+{
+    size_t len = strlen (s);
+    char *ret;
+
+    if (len <= n)
+	return strdup (s);
+
+    ret = malloc(n + 1);
+    strncpy(ret, s, n);
+    ret[n] = '\0';
+    return ret;
+}
+
+/* getline implementation is copied from glibc. */
+
+#ifndef SIZE_MAX
+# define SIZE_MAX ((size_t) -1)
+#endif
+#ifndef SSIZE_MAX
+# define SSIZE_MAX ((ssize_t) (SIZE_MAX / 2))
+#endif
+
+ssize_t getline (char **lineptr, size_t *n, FILE *fp)
+{
+    ssize_t result;
+    size_t cur_len = 0;
+
+    if (lineptr == NULL || n == NULL || fp == NULL)
+    {
+	errno = EINVAL;
+	return -1;
+    }
+
+    if (*lineptr == NULL || *n == 0)
+    {
+	*n = 120;
+	*lineptr = (char *) malloc (*n);
+	if (*lineptr == NULL)
+	{
+	    result = -1;
+	    goto end;
+	}
+    }
+
+    for (;;)
+    {
+	int i;
+
+	i = getc (fp);
+	if (i == EOF)
+	{
+	    result = -1;
+	    break;
+	}
+
+	/* Make enough space for len+1 (for final NUL) bytes.  */
+	if (cur_len + 1 >= *n)
+	{
+	    size_t needed_max =
+		SSIZE_MAX < SIZE_MAX ? (size_t) SSIZE_MAX + 1 : SIZE_MAX;
+	    size_t needed = 2 * *n + 1;   /* Be generous. */
+	    char *new_lineptr;
+
+	    if (needed_max < needed)
+		needed = needed_max;
+	    if (cur_len + 1 >= needed)
+	    {
+		result = -1;
+		goto end;
+	    }
+
+	    new_lineptr = (char *) realloc (*lineptr, needed);
+	    if (new_lineptr == NULL)
+	    {
+		result = -1;
+		goto end;
+	    }
+
+	    *lineptr = new_lineptr;
+	    *n = needed;
+	}
+
+	(*lineptr)[cur_len] = i;
+	cur_len++;
+
+	if (i == '\n')
+	    break;
+    }
+    (*lineptr)[cur_len] = '\0';
+    result = cur_len ? (ssize_t) cur_len : result;
+
+end:
+    return result;
+}
+#endif
+
 char *
 xstrndup (const char *s, size_t n)
 {
diff --git a/lib/xutil.h b/lib/xutil.h
index b973f7d..0b53f7c 100644
--- a/lib/xutil.h
+++ b/lib/xutil.h
@@ -25,6 +25,12 @@
 #include <sys/types.h>
 #include <regex.h>
 
+#ifdef __APPLE__
+/* Mac OS X don't have strndup and getline even if _GNU_SOURCE is defined */
+char *strndup (const char *s, size_t n);
+ssize_t getline(char **lineptr, size_t *n, FILE *stream);
+#endif
+
 /* xutil.c */
 void *
 xcalloc (size_t nmemb, size_t size);
-- 
1.6.4

Re: [PATCH] Mac OS X compatibility fixes

[Alexander Botero-Lowry]

On Sun, 22 Nov 2009 11:07:36 +0800, Jjgod Jiang <gzjjgod@gmail.com> wrote:
> Add missing GNU extensions strdup() and getline(). The C library
> shipped with Mac OS X does not include them (though it does support
> some GNU extensions when _GNU_SOURCE is defined), so we have to
> add these two. The getline() implementation is a modified version
> of getdelim() from GNU Libc.
Awesome!

> diff --git a/lib/xutil.c b/lib/xutil.c
> index 6fa5eb0..805b236 100644
> --- a/lib/xutil.c
> +++ b/lib/xutil.c
> @@ -79,6 +79,105 @@ xstrdup (const char *s)
>      return ret;
>  }
>  
> +#ifdef __APPLE__
Not awesome.

This should be done in a capabilites way, for example strndup was added
to FreeBSD in 7.2 (which is this current release of the 7 line), and so
for older versions of FreeBSD strndup will be needed. getdelim() and
getline() came in FreeBSD 8, so they'll be needed for the entire 7 line.
So Instead of just assuming __APPLE__ this should be done by determing
if the symbols are generally needed.

alex

Re: [PATCH] Mac OS X compatibility fixes

[Jjgod Jiang]

Hi Alex,

On Sun, Nov 22, 2009 at 12:17 PM, Alexander Botero-Lowry
<alex.boterolowry@gmail.com> wrote:
>> +#ifdef __APPLE__
> Not awesome.
>
> This should be done in a capabilites way, for example strndup was added
> to FreeBSD in 7.2 (which is this current release of the 7 line), and so
> for older versions of FreeBSD strndup will be needed. getdelim() and
> getline() came in FreeBSD 8, so they'll be needed for the entire 7 line.
> So Instead of just assuming __APPLE__ this should be done by determing
> if the symbols are generally needed.

The problem is that notmuch does not have a fully functional configure
process yet, Carl did mention Makefile.local, but it seems this file is
not generated by configure right now. (No config.h either.)

I will be happy to fix my patch if such facility (like
AC_CHECK_FUNCS([getline])) exists.

- Jiang

Re: [PATCH] Mac OS X compatibility fixes

[Carl Worth]

On Sun, 22 Nov 2009 12:31:53 +0800, Jjgod Jiang <gzjjgod@gmail.com> wrote:
> On Sun, Nov 22, 2009 at 12:17 PM, Alexander Botero-Lowry
> 
> The problem is that notmuch does not have a fully functional configure
> process yet, Carl did mention Makefile.local, but it seems this file is
> not generated by configure right now. (No config.h either.)

We do now have Makefile.config being generated, (and we can move to
config.h to avoid the overly-long command-line problem if necessary).

> I will be happy to fix my patch if such facility (like
> AC_CHECK_FUNCS([getline])) exists.

Chris, do you want to tackle this one? ;-)

A really easy hack would be to detect glibc, and if present, use all the
existing code in notmuch. If not present, then use all the compatibility
functions. This could be by building a notmuch-compat.c that defines
functions like _notmuch_strndup() and then adding thins like:

	#define strndup _notmuch_strndup.

That would allow our current configure script to stay quite simple. If
people want more sophistication we could write functions in the
configuration script for doing test compiles to find functions, etc.

-Carl

Re: [PATCH] Mac OS X compatibility fixes

[Jeffrey C. Ollie]

Here's a patch that adds private implementations of strndup and
getline.  They are unconditionally compiled so that compiler errors in
these functions can be detected on any platform, even those that
provide strndup and getline in the standard library.  I'll have a
patch that handles the strndup/getline detection later today (I've
already done it in SCons, just need to figure out how I want to do it
in plain shell for the current configure script).

[PATCH] Add private implementations of strndup and getline.

[Jeffrey C. Ollie]

Add private implementations of strndup and getline for those platforms
that don't have them (notably Mac OS X) no matter what preprocessor
symbols you define.

Signed-off-by: Jeffrey C. Ollie <jeff@ocjtech.us>
---
 lib/xutil.c |   99 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/xutil.h |    6 +++
 2 files changed, 105 insertions(+), 0 deletions(-)

diff --git a/lib/xutil.c b/lib/xutil.c
index 6fa5eb0..61467f1 100644
--- a/lib/xutil.c
+++ b/lib/xutil.c
@@ -79,6 +79,105 @@ xstrdup (const char *s)
     return ret;
 }
 
+/* Mac OS X don't have strndup even if _GNU_SOURCE is defined */
+char *
+_notmuch_strndup (const char *s, size_t n)
+{
+    size_t len = strlen (s);
+    char *ret;
+
+    if (len <= n)
+       return strdup (s);
+
+    ret = malloc(n + 1);
+    strncpy(ret, s, n);
+    ret[n] = '\0';
+    return ret;
+}
+
+/* getline implementation is copied from glibc. */
+
+#ifndef SIZE_MAX
+# define SIZE_MAX ((size_t) -1)
+#endif
+#ifndef SSIZE_MAX
+# define SSIZE_MAX ((ssize_t) (SIZE_MAX / 2))
+#endif
+
+ssize_t
+_notmuch_getline (char **lineptr, size_t *n, FILE *fp)
+{
+    ssize_t result;
+    size_t cur_len = 0;
+
+    if (lineptr == NULL || n == NULL || fp == NULL)
+    {
+       errno = EINVAL;
+       return -1;
+    }
+
+    if (*lineptr == NULL || *n == 0)
+    {
+       *n = 120;
+       *lineptr = (char *) malloc (*n);
+       if (*lineptr == NULL)
+       {
+           result = -1;
+           goto end;
+       }
+    }
+
+    for (;;)
+    {
+       int i;
+
+       i = getc (fp);
+       if (i == EOF)
+       {
+           result = -1;
+           break;
+       }
+
+       /* Make enough space for len+1 (for final NUL) bytes.  */
+       if (cur_len + 1 >= *n)
+       {
+           size_t needed_max =
+               SSIZE_MAX < SIZE_MAX ? (size_t) SSIZE_MAX + 1 : SIZE_MAX;
+           size_t needed = 2 * *n + 1;   /* Be generous. */
+           char *new_lineptr;
+
+           if (needed_max < needed)
+               needed = needed_max;
+           if (cur_len + 1 >= needed)
+           {
+               result = -1;
+               goto end;
+           }
+
+           new_lineptr = (char *) realloc (*lineptr, needed);
+           if (new_lineptr == NULL)
+           {
+               result = -1;
+               goto end;
+           }
+
+           *lineptr = new_lineptr;
+           *n = needed;
+       }
+
+       (*lineptr)[cur_len] = i;
+       cur_len++;
+
+       if (i == '\n')
+           break;
+    }
+    (*lineptr)[cur_len] = '\0';
+    result = cur_len ? (ssize_t) cur_len : result;
+
+end:
+    return result;
+}
+
 char *
 xstrndup (const char *s, size_t n)
 {
diff --git a/lib/xutil.h b/lib/xutil.h
index b973f7d..d13259a 100644
--- a/lib/xutil.h
+++ b/lib/xutil.h
@@ -39,6 +39,12 @@ char *
 xstrdup (const char *s);
 
 char *
+_notmuch_strndup (const char *s, size_t n);
+
+ssize_t
+_notmuch_getline (char **lineptr, size_t *n, FILE *stream);
+
+char *
 xstrndup (const char *s, size_t n);
 
 void
-- 
1.6.5.2

[PATCH] Add tests to configure script to detect strndup and getline

[Jeffrey C. Ollie]

Add some simple tests to the configure script to detect strndup and
getline.  It's not important that the tests run, just that they
compile and link without any errors.

Signed-off-by: Jeffrey C. Ollie <jeff@ocjtech.us>
---
 configure     |   20 +++++++++++++++++++-
 getlinetest.c |   13 +++++++++++++
 strnduptest.c |   10 ++++++++++
 3 files changed, 42 insertions(+), 1 deletions(-)
 create mode 100644 getlinetest.c
 create mode 100644 strnduptest.c

diff --git a/configure b/configure
index b4770ec..44c1700 100755
--- a/configure
+++ b/configure
@@ -118,6 +118,24 @@ EOF
     exit 1
 fi
 
+if ! gcc -o strnduptest strnduptest.c > /dev/null 2>&1
+then
+    echo "Checking for strndup... No."
+    strndup=-Dstrndup=_notmuch_strndup
+else
+    echo "Checking for strndup... Yes."
+fi
+rm -f strnduptest
+
+if ! gcc -o getlinetest getlinetest.c > /dev/null 2>&1
+then
+    echo "Checking for getline... No."
+    getline=-Dgetline=_notmuch_getline
+else
+    echo "Checking for getline... Yes."
+fi
+rm -f getlinetest
+
 cat <<EOF
 
 All required packages were found. You may now run the following
@@ -132,5 +150,5 @@ EOF
 cat > Makefile.config <<EOF
 prefix = /usr/local
 bash_completion_dir = /etc/bash_completion.d
-CFLAGS += ${have_valgrind}
+CFLAGS += ${have_valgrind} ${strndup} ${getline}
 EOF
diff --git a/getlinetest.c b/getlinetest.c
new file mode 100644
index 0000000..2a21a50
--- /dev/null
+++ b/getlinetest.c
@@ -0,0 +1,13 @@
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <sys/types.h>
+
+int main()
+{
+  ssize_t count = 0;
+  size_t n = 0;
+  char **lineptr = NULL;
+  FILE *stream = NULL;
+
+  count = getline(lineptr, &n, stream);
+}
diff --git a/strnduptest.c b/strnduptest.c
new file mode 100644
index 0000000..97c7c80
--- /dev/null
+++ b/strnduptest.c
@@ -0,0 +1,10 @@
+#include <string.h>
+
+int main()
+{
+  char *d;
+  const char *s = "";
+  size_t n = 0;
+
+  d = strndup(s, n);
+}
-- 
1.6.5.2

Re: [PATCH] Add tests to configure script to detect strndup and getline

[Marten Veldthuis]

On Mon, Nov 23, 2009 at 12:14:15PM -0600, Jeffrey C. Ollie wrote:
> cat > Makefile.config <<EOF
> prefix = /usr/local
> bash_completion_dir = /etc/bash_completion.d
>-CFLAGS += ${have_valgrind}
>+CFLAGS += ${have_valgrind} ${strndup} ${getline}
> EOF

This doesn't seem to do much for me, they don't seem to end up in the args 
passed to g++. I'm no Makefile wizard, so I got notmuch finally compiling by 
simply copy/pasting the ${strndup} and ${getline} from Makefile.config to 
Makefile:

# Now smash together user's values with our extra values
override CFLAGS += $(WARN_FLAGS) $(extra_cflags) -Dstrndup=_notmuch_strndup 
-Dgetline=_notmuch_getline
override CXXFLAGS += $(WARN_FLAGS) $(extra_cflags) $(extra_cxxflags) 
-Dstrndup=_notmuch_strndup -Dgetline=_notmuch_getline

-- 
- Marten

Re: [PATCH] Add private implementations of strndup and getline.

[Carl Worth]

On Mon, 23 Nov 2009 07:43:30 -0600, "Jeffrey C. Ollie" <jeff@ocjtech.us> wrote:
> Add private implementations of strndup and getline for those platforms
> that don't have them (notably Mac OS X) no matter what preprocessor
> symbols you define.

Thanks. This is off to a very good start.

> +char *
> +_notmuch_strndup (const char *s, size_t n)
> +{
> +    size_t len = strlen (s);
> +    char *ret;
> +
> +    if (len <= n)
> +       return strdup (s);
> +
> +    ret = malloc(n + 1);

This needs to check and return NULL if malloc returns NULL.

> +/* getline implementation is copied from glibc. */

Then, this should have added a copyright attribution to the top of the
file.

In fact, everyone that's contributing non-trivial amounts should be
adding copyright statements. I don't mind too much if people consider
their own contributions to be insignificant enough to not merit a
copyright claim, but when copying code written by others, we definitely
need to bring the copyright attribution along.

Would also be good to mention the glibc version and license in this
comment and in the commit message as well.

-Carl