lkml.org 
[lkml]   [2009]   [Oct]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 10/19] workqueue: update cwq alignement and make one more flag bit available
(Restoring cc list.  Please don't drop them)

Ben Pfaff wrote:
> David Howells <dhowells@redhat.com> writes:
>
>> Tejun Heo <tj@kernel.org> wrote:
>>
>>> +enum {
>>> + WORK_STRUCT_PENDING = 0, /* work item is pending execution */
>>> +
>>> + /*
>>> + * Reserve 3bits off of cwq pointer. This is enough and
>>> + * provides acceptable alignment on both 32 and 64bit
>>> + * machines.
>>> + */
>>> + WORK_STRUCT_FLAG_BITS = 3,
>>> +
>>> + WORK_STRUCT_FLAG_MASK = (1UL << WORK_STRUCT_FLAG_BITS) - 1,
>>> + WORK_STRUCT_WQ_DATA_MASK = ~WORK_STRUCT_FLAG_MASK,
>>> +};
>> There's some great enum abuse going on here:-)
>
> The "1UL" part is a bit worrisome. Enumeration constants always
> have type "int"[*], so if code that uses WORK_STRUCT_WQ_DATA_MASK
> actually depends on getting a full "long" worth of bits, it is
> not going to work on 64-bit systems.
>
> [*] See C99:
>
> 6.4.4.3 Enumeration constants
> Syntax
> 1 enumeration-constant:
> identifier
> Semantics
> 2 An identifier declared as an enumeration constant has type int.

Aieee... oops. Well, this isn't how gcc behaves.

$ cat test.c
#include <stdio.h>

enum {
ENUM = ~0U,
};

enum {
LONG_ENUM = ~0UL,
};

int main(void)
{
printf("%zu %zu\n", sizeof(ENUM), sizeof(LONG_ENUM));
return 0;
}
$ gcc test.c && ./a.out; gcc -std=c99 test.c && ./a.out
4 8
4 8

But, yeah, this definitely is a problem. 6.7.2.2 also says that

Each enumerated type shall be compatible with char, a signed integer
type, or an unsigned integer type. The choice of type is
implementation-defined,113) but shall be capable of representing the
values of all the members of the enumeration. The enumerated type is
incomplete until after the } that terminates the list of enumerator
declarations.

gcc probably is being a bit too generous with possible integer types
here. BTW, does c++ define it like this too?

Hmmm... So, should we go back to using defines for these or keep
(ab)using gcc's generousity and maybe hope the next iteration of the
standard to become a bit more generous too?

Thanks.

--
tejun


\
 
 \ /
  Last update: 2009-10-02 14:01    [W:0.107 / U:8.924 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site