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

Revision history for this message
Brian Aker (brianaker) 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.

« Back to merge proposal