On Wed, 20.08.08 03:12, Brian Cameron ([EMAIL PROTECTED]) wrote: Heya!
> > In order to get libcanberra to build on Solaris, I had to apply the > attached patch. > > - Note that the src/Makefile.am is hardcoded to include some GCC > specific options. This part of the patch is probably not ready to > go upstream. Might be better to check whether GCC is being used > in configure, and then only add the map-file stuff to LDFLAGS when > GCC is actually being used. This has already been fixed properly in the latest release (0.7). http://git.0pointer.de/?p=libcanberra.git;a=commitdiff;h=0f3825b0c40ca379e6b5d2e3e862130c8ccb65f5 > - In order for var-args stuff to build on Solaris, it is necessary to > add "#include <varargs.h>" to src/common.h I am a bit surprised that this is sufficient. Source files like proplist.c use va_ functions, but don't include either src/common.h nor varags.h themselves. varargs.h is considered "legacy" by Single UNIX/POSIX. Are you sure that Solaris doesn't have stdargs.h like every other ANSI C89 implementation? If so, could you please fix this patch to include stdargs.h instead of varargs.h? > - Solaris doesn't have strndup, so the attached patch adds it to > src/sound-theme-spec.c if it isn't already defined. Could you please move this to malloc.[ch]? The reason we have malloc.[ch] is explicitly to allow implementation of ca_strndup() as a macro where strndup is supported and as function where it is not. I'd prefer not to pollute unrelated code with implementations with strndup if we don't have to. Also, I don't like the implementation of your strndup(). See below. > libcanberra_la_LDFLAGS = \ > - -export-dynamic \ > - -version-info $(LIBCANBERRA_VERSION_INFO) \ > - -Wl,-version-script=$(srcdir)/map-file > + -export-dynamic You are deleting too much here. version-info is necessary! > @@ -215,7 +213,7 @@ libcanberra_gtk_la_LIBADD = \ > $(GTK_LIBS) \ > libcanberra.la > libcanberra_gtk_la_LDFLAGS = \ > - -export-dynamic -version-info $(LIBCANBERRA_GTK_VERSION_INFO) > + -export-dynamic $(LIBCANBERRA_GTK_VERSION_INFO) Uh? This hunk makes no sense. > #define DEFAULT_OUTPUT_PROFILE "stereo" > #define N_THEME_DIR_MAX 8 > > +#ifndef strndup > +static char *strndup (const char *s, size_t n) > +{ > + size_t length; > + char *ret; > + > + if (!s) > + return 0; NULL should be returned here, not 0. > + > + if (strlen(s)+1 < n+1) > + length = strlen(s)+1; > + else > + length = n+1; Calling strlen twice could be avoided here. > + > + ret = malloc (length * sizeof (char)); > + > + memcpy (ret, s, length); In case of OOM this will segfault. Please check for the return value of malloc() and return the NULL properly. Anyway, thanks for the patches. If you fix the issues I pointed out above I will happily merge them. Oh, one last favour. If you resubmit this, could you please write one email per patch? This would make it easier for me to attribute the patches to you in git while still keeping them seperate commits. Thanks, Lennart -- Lennart Poettering Red Hat, Inc. lennart [at] poettering [dot] net ICQ# 11060553 http://0pointer.net/lennart/ GnuPG 0x1A015CC4 _______________________________________________ libcanberra-discuss mailing list [email protected] https://tango.0pointer.de/mailman/listinfo/libcanberra-discuss
