linux-kernel - Re: [PATCH 10/19] workqueue: update cwq alignement and make one more flag bit available

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 02 Oct 2009 20:56:23 +0900
From:	Tejun Heo <tj@...nel.org>
To:	blp@...stanford.edu
CC:	linux-kernel@...r.kernel.org, Jeff Garzik <jeff@...zik.org>,
	Alan Cox <alan@...rguk.ukuu.org.uk>,
	David Howells <dhowells@...hat.com>,
	Ingo Molnar <mingo@...e.hu>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Jens Axboe <jens.axboe@...cle.com>,
	Rusty Russell <rusty@...tcorp.com.au>,
	Christoph Lameter <cl@...ux-foundation.org>,
	Arjan van de Ven <arjan@...radead.org>
Subject: Re: [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@...hat.com> writes:
> 
>> Tejun Heo <tj@...nel.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/