Code review comment for lp:~eday/gearmand/fix-struct-alignment

Revision history for this message
Eric Day (eday) wrote :

Ok, branch updated with default from only Linux to not Solaris.

On Tue, Mar 09, 2010 at 11:21:19PM -0000, Brian Aker wrote:
> Ok, just disable optimizations on Solaris and pull the macro.
>
> On Mar 9, 2010, at 3:15 PM, Eric Day wrote:
>
> > I don't really have a preference how this is fixed, this just seemed
> > like a generic way we can control from pandora-build if we want to
> > fine-tune it. We just can't have any public headers depending on
> > config.h like they do now.
> >
> > -Eric
> >
> > On Tue, Mar 09, 2010 at 11:09:21PM -0000, Brian Aker wrote:
> >> The other option to this is just to turn down the optimization on
> >> Solaris (since that is where the bug is).
> >>
> >>
> >> On Mar 9, 2010, at 3:07 PM, Eric Day wrote:
> >>
> >>> Eric Day has proposed merging lp:~eday/gearmand/fix-struct-alignment
> >>> into lp:gearmand.
> >>>
> >>> Requested reviews:
> >>> Gearman-developers (gearman-developers)
> >>>
> >>> --
> >>> https://code.launchpad.net/~eday/gearmand/fix-struct-alignment/+merge/21012
> >>> Your team Gearman-developers is subscribed to branch lp:gearmand.
> >>> === modified file 'libgearman/configure.h.in'
> >>> --- libgearman/configure.h.in 2010-02-12 20:26:31 +0000
> >>> +++ libgearman/configure.h.in 2010-03-09 23:07:17 +0000
> >>> @@ -16,7 +16,9 @@
> >>> extern "C" {
> >>> #endif
> >>>
> >>> -#ifdef TARGET_OS_LINUX
> >>> +#define LIBGEARMAN_OPTIMIZE_BITFIELD @PANDORA_OPTIMIZE_BITFIELD@
> >>> +
> >>> +#if LIBGEARMAN_OPTIMIZE_BITFIELD == 1
> >>> #define LIBGEARMAN_BITFIELD :1
> >>> #else
> >>> #define LIBGEARMAN_BITFIELD
> >>>
> >>> === modified file 'libgearman/connection.c'
> >>> --- libgearman/connection.c 2010-01-28 23:46:52 +0000
> >>> +++ libgearman/connection.c 2010-03-09 23:07:17 +0000
> >>> @@ -115,7 +115,7 @@
> >>> {
> >>> connection= gearman_connection_create(gearman, connection, NULL);
> >>>
> >>> - if (from || connection == NULL)
> >>> + if (from == NULL || connection == NULL)
> >>> return connection;
> >>>
> >>> connection->options.ready= from->options.ready;
> >>>
> >>> === modified file 'm4/pandora_platform.m4'
> >>> --- m4/pandora_platform.m4 2010-01-02 05:06:13 +0000
> >>> +++ m4/pandora_platform.m4 2010-03-09 23:07:17 +0000
> >>> @@ -34,11 +34,14 @@
> >>> ;;
> >>> esac
> >>>
> >>> + PANDORA_OPTIMIZE_BITFIELD=0
> >>> +
> >>> case "$target_os" in
> >>> *linux*)
> >>> TARGET_LINUX="true"
> >>> AC_SUBST(TARGET_LINUX)
> >>> AC_DEFINE([TARGET_OS_LINUX], [1], [Whether we build for Linux])
> >>> + PANDORA_OPTIMIZE_BITFIELD=1
> >>> ;;
> >>> *darwin*)
> >>> TARGET_OSX="true"
> >>> @@ -60,6 +63,8 @@
> >>> ;;
> >>> esac
> >>>
> >>> + AC_SUBST(PANDORA_OPTIMIZE_BITFIELD)
> >>> +
> >>> AC_CHECK_DECL([__SUNPRO_C], [SUNCC="yes"], [SUNCC="no"])
> >>> AC_CHECK_DECL([__ICC], [INTELCC="yes"], [INTELCC="no"])
> >>>
> >>>
> >>> === modified file 'tests/client_test.c'
> >>> --- tests/client_test.c 2010-01-28 21:45:14 +0000
> >>> +++ tests/client_test.c 2010-03-09 23:07:17 +0000
> >>> @@ -99,6 +99,7 @@
> >>> test_return_t clone_test(void *object)
> >>> {
> >>> const gearman_client_st *from= (gearman_client_st *)object;
> >>> + gearman_client_st *from_with_host;
> >>> gearman_client_st *client;
> >>>
> >>> client= gearman_client_clone(NULL, NULL);
> >>> @@ -110,8 +111,21 @@
> >>>
> >>> client= gearman_client_clone(NULL, from);
> >>> test_truth(client);
> >>> -
> >>> - gearman_client_free(client);
> >>> + gearman_client_free(client);
> >>> +
> >>> + from_with_host= gearman_client_create(NULL);
> >>> + test_truth(from_with_host);
> >>> + gearman_client_add_server(from_with_host, "127.0.0.1", 12345);
> >>> +
> >>> + client= gearman_client_clone(NULL, from_with_host);
> >>> + test_truth(client);
> >>> +
> >>> + test_truth(client->universal.con_list);
> >>> + test_truth(!strcmp(client->universal.con_list->host,
> >>> from_with_host->universal.con_list->host));
> >>> + test_truth(client->universal.con_list->port == from_with_host-
> >>>> universal.con_list->port);
> >>> +
> >>> + gearman_client_free(client);
> >>> + gearman_client_free(from_with_host);
> >>>
> >>> return TEST_SUCCESS;
> >>> }
> >>>
> >>
> >> --
> >> https://code.launchpad.net/~eday/gearmand/fix-struct-alignment/+merge/21012
> >> You are the owner of lp:~eday/gearmand/fix-struct-alignment.
> > --
> > https://code.launchpad.net/~eday/gearmand/fix-struct-alignment/+merge/21012
> > Your team Gearman-developers is subscribed to branch lp:gearmand.
>
> --
> https://code.launchpad.net/~eday/gearmand/fix-struct-alignment/+merge/21012
> You are the owner of lp:~eday/gearmand/fix-struct-alignment.

« Back to merge proposal