From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by olra.theworths.org (Postfix) with ESMTP id A6115431FD0 for ; Mon, 31 Oct 2011 02:20:37 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org X-Spam-Flag: NO X-Spam-Score: -0.7 X-Spam-Level: X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5 tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled Received: from olra.theworths.org ([127.0.0.1]) by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id WQkfKg5nrbL0 for ; Mon, 31 Oct 2011 02:20:36 -0700 (PDT) Received: from mail-qw0-f53.google.com (mail-qw0-f53.google.com [209.85.216.53]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (No client certificate requested) by olra.theworths.org (Postfix) with ESMTPS id EAE0B431FB6 for ; Mon, 31 Oct 2011 02:20:19 -0700 (PDT) Received: by qadc1 with SMTP id c1so6275169qad.26 for ; Mon, 31 Oct 2011 02:20:19 -0700 (PDT) Received: by 10.224.175.197 with SMTP id bb5mr11220679qab.60.1320052819181; Mon, 31 Oct 2011 02:20:19 -0700 (PDT) Received: from localhost (nikula.org. [92.243.24.172]) by mx.google.com with ESMTPS id h2sm29907903qad.18.2011.10.31.02.20.15 (version=SSLv3 cipher=OTHER); Mon, 31 Oct 2011 02:20:17 -0700 (PDT) From: Jani Nikula To: David Bremner , notmuch@notmuchmail.org Subject: Re: [PATCH] xutil.c: remove duplicate copies, create new library libutil.a to contain xutil. In-Reply-To: <1319383133-11006-1-git-send-email-david@tethera.net> References: <1319383133-11006-1-git-send-email-david@tethera.net> User-Agent: Notmuch/0.5-232-g917e874 (http://notmuchmail.org) Emacs/23.1.1 (i686-pc-linux-gnu) Date: Mon, 31 Oct 2011 09:20:13 +0000 Message-ID: <877h3ly0tu.fsf@nikula.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Cc: David Bremner X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 31 Oct 2011 09:20:37 -0000 Hi David, this patch (commit 1dedfc90f6eee7cad10f1a1ceb39a7a1c4dbd1b1) broke my build with: CXX -O2 lib/libnotmuch.so.2.0.0 lib/message-file.o: In function `copy_header_unfolding': message-file.c:(.text+0xea): undefined reference to `xrealloc' lib/message-file.o: In function `notmuch_message_file_restrict_headersv': message-file.c:(.text+0x2cc): undefined reference to `xstrdup' lib/message-file.o: In function `notmuch_message_file_get_header': message-file.c:(.text+0x4af): undefined reference to `xstrndup' message-file.c:(.text+0x5c8): undefined reference to `xmalloc' lib/messages.o: In function `notmuch_messages_collect_tags': messages.c:(.text+0x1c4): undefined reference to `xstrdup' lib/sha1.o: In function `_hex_of_sha1_digest': sha1.c:(.text+0x18): undefined reference to `xcalloc' lib/thread.o: In function `_notmuch_thread_create': thread.cc:(.text+0x34e): undefined reference to `xstrdup' thread.cc:(.text+0x58c): undefined reference to `xstrdup' /usr/bin/ld: lib/libnotmuch.so.2.0.0: hidden symbol `xmalloc' isn't defined /usr/bin/ld: final link failed: Bad value collect2: ld returned 1 exit status make: *** [lib/libnotmuch.so.2.0.0] Error 1 I don't have the time to debug this further right now, but I bisected the failure to this commit and did a clean clone to make sure leftover files weren't the cause. BR, Jani. On Sun, 23 Oct 2011 12:18:53 -0300, David Bremner wrote: > From: David Bremner >=20 > We keep the lib/xutil.c version. As a consequence, also factor out > _internal_error and associated macros. It might be overkill to make a > new file error_util.c for this, but _internal_error does not really > belong in database.cc. > --- >=20 > This turned out to be more disruptive than I thought. On the other > hand, having two copies of xutil.c seems like a recipe for disaster. > I wanted to factor out the logic in xregcomp so I could use it in > situations where miscompilation is not an internal error, but more > likely a user error. >=20 > Makefile | 2 +- > Makefile.local | 7 +-- > lib/Makefile.local | 5 +- > lib/database.cc | 15 ----- > lib/notmuch-private.h | 20 +------- > lib/xutil.c | 134 -------------------------------------------= ---- > lib/xutil.h | 55 ------------------- > util/Makefile | 5 ++ > util/Makefile.local | 11 ++++ > util/error_util.c | 41 +++++++++++++++ > util/error_util.h | 45 ++++++++++++++++ > util/xutil.c | 136 +++++++++++++++++++++++++++++++++++++++++++= +++++ > util/xutil.h | 55 +++++++++++++++++++ > xutil.c | 138 -------------------------------------------= ------ > 14 files changed, 300 insertions(+), 369 deletions(-) > delete mode 100644 lib/xutil.c > delete mode 100644 lib/xutil.h > create mode 100644 util/Makefile > create mode 100644 util/Makefile.local > create mode 100644 util/error_util.c > create mode 100644 util/error_util.h > create mode 100644 util/xutil.c > create mode 100644 util/xutil.h > delete mode 100644 xutil.c >=20 > diff --git a/Makefile b/Makefile > index 11e3a3d..2fb2a61 100644 > --- a/Makefile > +++ b/Makefile > @@ -3,7 +3,7 @@ > all: >=20=20 > # List all subdirectories here. Each contains its own Makefile.local > -subdirs =3D compat completion emacs lib test > +subdirs =3D compat completion emacs lib util test >=20=20 > # We make all targets depend on the Makefiles themselves. > global_deps =3D Makefile Makefile.config Makefile.local \ > diff --git a/Makefile.local b/Makefile.local > index 38f6c17..e31defa 100644 > --- a/Makefile.local > +++ b/Makefile.local > @@ -39,7 +39,7 @@ PV_FILE=3Dbindings/python/notmuch/version.py > # Smash together user's values with our extra values > FINAL_CFLAGS =3D -DNOTMUCH_VERSION=3D$(VERSION) $(CFLAGS) $(WARN_CFLAGS)= $(CONFIGURE_CFLAGS) $(extra_cflags) > FINAL_CXXFLAGS =3D $(CXXFLAGS) $(WARN_CXXFLAGS) $(CONFIGURE_CXXFLAGS) $(= extra_cflags) $(extra_cxxflags) > -FINAL_NOTMUCH_LDFLAGS =3D $(LDFLAGS) -Llib -lnotmuch $(AS_NEEDED_LDFLAGS= ) $(GMIME_LDFLAGS) $(TALLOC_LDFLAGS) > +FINAL_NOTMUCH_LDFLAGS =3D $(LDFLAGS) -Lutil -lutil -Llib -lnotmuch $(AS_= NEEDED_LDFLAGS) $(GMIME_LDFLAGS) $(TALLOC_LDFLAGS) > FINAL_NOTMUCH_LINKER =3D CC > ifneq ($(LINKER_RESOLVES_LIBRARY_DEPENDENCIES),1) > FINAL_NOTMUCH_LDFLAGS +=3D $(CONFIGURE_LDFLAGS) > @@ -288,12 +288,11 @@ notmuch_client_srcs =3D \ > notmuch-time.c \ > query-string.c \ > show-message.c \ > - json.c \ > - xutil.c > + json.c=09=09=09 >=20=20 > notmuch_client_modules =3D $(notmuch_client_srcs:.c=3D.o) >=20=20 > -notmuch: $(notmuch_client_modules) lib/libnotmuch.a > +notmuch: $(notmuch_client_modules) lib/libnotmuch.a util/libutil.a > $(call quiet,CXX $(CFLAGS)) $^ $(FINAL_LIBNOTMUCH_LDFLAGS) -o $@ >=20=20 > notmuch-shared: $(notmuch_client_modules) lib/$(LINKER_NAME) > diff --git a/lib/Makefile.local b/lib/Makefile.local > index ea20b2b..f148661 100644 > --- a/lib/Makefile.local > +++ b/lib/Makefile.local > @@ -49,8 +49,7 @@ libnotmuch_c_srcs =3D \ > $(dir)/message-file.c \ > $(dir)/messages.c \ > $(dir)/sha1.c \ > - $(dir)/tags.c \ > - $(dir)/xutil.c > + $(dir)/tags.c=09=09 >=20=20 > libnotmuch_cxx_srcs =3D \ > $(dir)/database.cc \ > @@ -66,7 +65,7 @@ $(dir)/libnotmuch.a: $(libnotmuch_modules) > $(call quiet,AR) rcs $@ $^ >=20=20 > $(dir)/$(LIBNAME): $(libnotmuch_modules) notmuch.sym > - $(call quiet,CXX $(CXXFLAGS)) $(libnotmuch_modules) $(FINAL_LIBNOTMUCH_= LDFLAGS) $(LIBRARY_LINK_FLAG) -o $@ > + $(call quiet,CXX $(CXXFLAGS)) $(libnotmuch_modules) $(FINAL_LIBNOTMUCH_= LDFLAGS) $(LIBRARY_LINK_FLAG) -o $@ -L$(srcdir)/util -lutil >=20=20 > notmuch.sym: lib/notmuch.h $(libnotmuch_modules) > sh lib/gen-version-script.sh $< $(libnotmuch_modules) > $@ > diff --git a/lib/database.cc b/lib/database.cc > index e77fd53..88be939 100644 > --- a/lib/database.cc > +++ b/lib/database.cc > @@ -209,21 +209,6 @@ static prefix_t PROBABILISTIC_PREFIX[]=3D { > { "folder", "XFOLDER"} > }; >=20=20 > -int > -_internal_error (const char *format, ...) > -{ > - va_list va_args; > - > - va_start (va_args, format); > - > - fprintf (stderr, "Internal error: "); > - vfprintf (stderr, format, va_args); > - > - exit (1); > - > - return 1; > -} > - > const char * > _find_prefix (const char *name) > { > diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h > index d319530..0d3cc27 100644 > --- a/lib/notmuch-private.h > +++ b/lib/notmuch-private.h > @@ -47,6 +47,7 @@ NOTMUCH_BEGIN_DECLS > #include >=20=20 > #include "xutil.h" > +#include "error_util.h" >=20=20 > #pragma GCC visibility push(hidden) >=20=20 > @@ -60,25 +61,6 @@ NOTMUCH_BEGIN_DECLS > #define STRNCMP_LITERAL(var, literal) \ > strncmp ((var), (literal), sizeof (literal) - 1) >=20=20 > -/* There's no point in continuing when we've detected that we've done > - * something wrong internally (as opposed to the user passing in a > - * bogus value). > - * > - * Note that PRINTF_ATTRIBUTE comes from talloc.h > - */ > -int > -_internal_error (const char *format, ...) PRINTF_ATTRIBUTE (1, 2); > - > -/* There's no point in continuing when we've detected that we've done > - * something wrong internally (as opposed to the user passing in a > - * bogus value). > - * > - * Note that __location__ comes from talloc.h. > - */ > -#define INTERNAL_ERROR(format, ...) \ > - _internal_error (format " (%s).\n", \ > - ##__VA_ARGS__, __location__) > - > #define unused(x) x __attribute__ ((unused)) >=20=20 > #ifdef __cplusplus > diff --git a/lib/xutil.c b/lib/xutil.c > deleted file mode 100644 > index 268225b..0000000 > diff --git a/lib/xutil.h b/lib/xutil.h > deleted file mode 100644 > index fd77f73..0000000 > diff --git a/util/Makefile b/util/Makefile > new file mode 100644 > index 0000000..fa25832 > --- /dev/null > +++ b/util/Makefile > @@ -0,0 +1,5 @@ > +all: > + $(MAKE) -C .. all > + > +.DEFAULT: > + $(MAKE) -C .. $@ > diff --git a/util/Makefile.local b/util/Makefile.local > new file mode 100644 > index 0000000..2ff42b3 > --- /dev/null > +++ b/util/Makefile.local > @@ -0,0 +1,11 @@ > +# -*- makefile -*- > + > +dir :=3D util > +extra_cflags +=3D -I$(srcdir)/$(dir) > + > +libutil_c_srcs :=3D $(dir)/xutil.c $(dir)/error_util.c > + > +libutil_modules :=3D $(libutil_c_srcs:.c=3D.o) > + > +$(dir)/libutil.a: $(libutil_modules) > + $(call quiet,AR) rcs $@ $^ > diff --git a/util/error_util.c b/util/error_util.c > new file mode 100644 > index 0000000..630d228 > --- /dev/null > +++ b/util/error_util.c > @@ -0,0 +1,41 @@ > +/* error_util.c - internal error utilities for notmuch. > + * > + * Copyright =C2=A9 2009 Carl Worth > + * > + * This program is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation, either version 3 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see http://www.gnu.org/licenses/ . > + * > + * Author: Carl Worth > + */ > + > +#include > +#include > +#include > + > +#include "error_util.h" > + > +int > +_internal_error (const char *format, ...) > +{ > + va_list va_args; > + > + va_start (va_args, format); > + > + fprintf (stderr, "Internal error: "); > + vfprintf (stderr, format, va_args); > + > + exit (1); > + > + return 1; > +} > + > diff --git a/util/error_util.h b/util/error_util.h > new file mode 100644 > index 0000000..0f1e5ef > --- /dev/null > +++ b/util/error_util.h > @@ -0,0 +1,45 @@ > +/* error_util.h - Internal interfaces for notmuch. > + * > + * Copyright =C2=A9 2009 Carl Worth > + * > + * This program is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation, either version 3 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see http://www.gnu.org/licenses/ . > + * > + * Author: Carl Worth > + */ > + > +#ifndef ERROR_UTIL_H > +#define ERROR_UTIL_H > + > +#include > + > +/* There's no point in continuing when we've detected that we've done > + * something wrong internally (as opposed to the user passing in a > + * bogus value). > + * > + * Note that PRINTF_ATTRIBUTE comes from talloc.h > + */ > +int > +_internal_error (const char *format, ...) PRINTF_ATTRIBUTE (1, 2); > + > +/* There's no point in continuing when we've detected that we've done > + * something wrong internally (as opposed to the user passing in a > + * bogus value). > + * > + * Note that __location__ comes from talloc.h. > + */ > +#define INTERNAL_ERROR(format, ...) \ > + _internal_error (format " (%s).\n", \ > + ##__VA_ARGS__, __location__) > + > +#endif > diff --git a/util/xutil.c b/util/xutil.c > new file mode 100644 > index 0000000..15ff765 > --- /dev/null > +++ b/util/xutil.c > @@ -0,0 +1,136 @@ > +/* xutil.c - Various wrapper functions to abort on error. > + * > + * Copyright =C2=A9 2009 Carl Worth > + * > + * This program is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation, either version 3 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see http://www.gnu.org/licenses/ . > + * > + * Author: Carl Worth > + */ > + > +#include > +#include > + > +#include "xutil.h" > +#include "error_util.h" > + > +void * > +xcalloc (size_t nmemb, size_t size) > +{ > + void *ret; > + > + ret =3D calloc (nmemb, size); > + if (ret =3D=3D NULL) { > + fprintf (stderr, "Out of memory.\n"); > + exit (1); > + } > + > + return ret; > +} > + > +void * > +xmalloc (size_t size) > +{ > + void *ret; > + > + ret =3D malloc (size); > + if (ret =3D=3D NULL) { > + fprintf (stderr, "Out of memory.\n"); > + exit (1); > + } > + > + return ret; > +} > + > +void * > +xrealloc (void *ptr, size_t size) > +{ > + void *ret; > + > + ret =3D realloc (ptr, size); > + if (ret =3D=3D NULL) { > + fprintf (stderr, "Out of memory.\n"); > + exit (1); > + } > + > + return ret; > +} > + > +char * > +xstrdup (const char *s) > +{ > + char *ret; > + > + ret =3D strdup (s); > + if (ret =3D=3D NULL) { > + fprintf (stderr, "Out of memory.\n"); > + exit (1); > + } > + > + return ret; > +} > + > +char * > +xstrndup (const char *s, size_t n) > +{ > + char *ret; > + > + if (strlen (s) <=3D n) > + n =3D strlen (s); > + > + ret =3D malloc (n + 1); > + if (ret =3D=3D NULL) { > + fprintf (stderr, "Out of memory.\n"); > + exit (1); > + } > + memcpy (ret, s, n); > + ret[n] =3D '\0'; > + > + return ret; > +} > + > +void > +xregcomp (regex_t *preg, const char *regex, int cflags) > +{ > + int rerr; > + > + rerr =3D regcomp (preg, regex, cflags); > + if (rerr) { > + size_t error_size =3D regerror (rerr, preg, NULL, 0); > + char *error =3D xmalloc (error_size); > + > + regerror (rerr, preg, error, error_size); > + INTERNAL_ERROR ("compiling regex %s: %s\n", > + regex, error); > + } > +} > + > +int > +xregexec (const regex_t *preg, const char *string, > + size_t nmatch, regmatch_t pmatch[], int eflags) > +{ > + unsigned int i; > + int rerr; > + > + rerr =3D regexec (preg, string, nmatch, pmatch, eflags); > + if (rerr) > + return rerr; > + > + for (i =3D 0; i < nmatch; i++) { > + if (pmatch[i].rm_so =3D=3D -1) > + INTERNAL_ERROR ("matching regex against %s: Sub-match %d not found\= n", > + string, i); > + } > + > + return 0; > +} > diff --git a/util/xutil.h b/util/xutil.h > new file mode 100644 > index 0000000..fd77f73 > --- /dev/null > +++ b/util/xutil.h > @@ -0,0 +1,55 @@ > +/* xutil.h - Various wrapper functions to abort on error. > + * > + * Copyright =C2=A9 2009 Carl Worth > + * > + * This program is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation, either version 3 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see http://www.gnu.org/licenses/ . > + * > + * Author: Carl Worth > + */ > + > +#ifndef NOTMUCH_XUTIL_H > +#define NOTMUCH_XUTIL_H > + > +#include > +#include > +#include > + > +#pragma GCC visibility push(hidden) > + > +/* xutil.c */ > +void * > +xcalloc (size_t nmemb, size_t size); > + > +void * > +xmalloc (size_t size); > + > +void * > +xrealloc (void *ptrr, size_t size); > + > +char * > +xstrdup (const char *s); > + > +char * > +xstrndup (const char *s, size_t n); > + > +void > +xregcomp (regex_t *preg, const char *regex, int cflags); > + > +int > +xregexec (const regex_t *preg, const char *string, > + size_t nmatch, regmatch_t pmatch[], int eflags); > + > +#pragma GCC visibility pop > + > +#endif > diff --git a/xutil.c b/xutil.c > deleted file mode 100644 > index 5f98f3f..0000000 > --=20 > 1.7.6.3 >=20 > _______________________________________________ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch