Re: [PATCH 1/2] afs: Move UUID struct to linux/uuid.h

[kbuild test robot]

Hi David,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.10-rc3 next-20170113]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/David-Howells/afs-Move-UUID-struct-to-linux-uuid-h/20170113-134212
config: x86_64-randconfig-b0-01131757 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

Note: the 
linux-review/David-Howells/afs-Move-UUID-struct-to-linux-uuid-h/20170113-134212 
HEAD c4259bc3968ea592af3a1599099327eb67e6019d builds fine.
  It only hurts bisectibility.

All error/warnings (new ones prefixed by >>):

   fs/afs/main.c: In function 'afs_get_client_UUID':
   fs/afs/main.c:48:2: error: invalid use of undefined type 'struct uuid_v1'
 ret = afs_get_MAC_address(afs_uuid.node, sizeof(afs_uuid.node));
 ^~~
   fs/afs/main.c:48:2: error: invalid use of undefined type 'struct uuid_v1'
   fs/afs/main.c:55:14: error: 'UUID_TO_UNIX_TIME' undeclared (first use in 
this function)
 uuidtime += UUID_TO_UNIX_TIME;
 ^
   fs/afs/main.c:55:14: note: each undeclared identifier is reported only once 
for each function it appears in
   fs/afs/main.c:56:2: error: invalid use of undefined type 'struct uuid_v1'
 afs_uuid.time_low = htonl(uuidtime);
 ^~~~
   fs/afs/main.c:57:2: error: invalid use of undefined type 'struct uuid_v1'
 afs_uuid.time_mid = htons(uuidtime >> 32);
 ^~~~
   fs/afs/main.c:58:28: error: 'UUID_TIMEHI_MASK' undeclared (first use in this 
function)
 hi_v = (uuidtime >> 48) & UUID_TIMEHI_MASK;
   ^~~~
   fs/afs/main.c:59:10: error: 'UUID_VERSION_TIME' undeclared (first use in 
this function)
 hi_v |= UUID_VERSION_TIME;
 ^
   fs/afs/main.c:60:2: error: invalid use of undefined type 'struct uuid_v1'
 afs_uuid.time_hi_and_version = htons(hi_v);
 ^~~~
   fs/afs/main.c:63:2: error: invalid use of undefined type 'struct uuid_v1'
 afs_uuid.clock_seq_low = clockseq;
 ^~~~
   fs/afs/main.c:64:2: error: invalid use of undefined type 'struct uuid_v1'
 afs_uuid.clock_seq_hi_and_reserved =
 ^~~~
   fs/afs/main.c:65:21: error: 'UUID_CLOCKHI_MASK' undeclared (first use in 
this function)
  (clockseq >> 8) & UUID_CLOCKHI_MASK;
^
   fs/afs/main.c:66:2: error: invalid use of undefined type 'struct uuid_v1'
 afs_uuid.clock_seq_hi_and_reserved |= UUID_VARIANT_STD;
 ^~~~
   fs/afs/main.c:66:40: error: 'UUID_VARIANT_STD' undeclared (first use in this 
function)
 afs_uuid.clock_seq_hi_and_reserved |= UUID_VARIANT_STD;
   ^~~~
   In file included from include/uapi/linux/stddef.h:1:0,
from include/linux/stddef.h:4,
from include/uapi/linux/posix_types.h:4,
from include/uapi/linux/types.h:13,
from include/linux/types.h:5,
from include/linux/list.h:4,
from include/linux/module.h:9,
from fs/afs/main.c:12:
>> include/linux/compiler.h:117:18: error: invalid use of undefined type 
>> 'struct uuid_v1'
   static struct ftrace_branch_data  \
 ^
   include/linux/compiler.h:139:58: note: in expansion of macro 
'__branch_check__'
#  define unlikely(x) (__builtin_constant_p(x) ? !!(x) : 
__branch_check__(x, 0))
 ^~~~
>> fs/afs/internal.h:769:6: note: in expansion of macro 'unlikely'
 if (unlikely(afs_debug & AFS_DEBUG_KDEBUG)) \
 ^~~~
   fs/afs/main.c:68:2: note: in expansion of macro '_debug'
 _debug("AFS UUID: %08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
 ^~
>> include/linux/compiler.h:117:18: error: invalid use of undefined type 
>> 'struct uuid_v1'
   static struct ftrace_branch_data  \
 ^
   include/linux/compiler.h:139:58: note: in expansion of macro 
'__branch_check__'
#  define unlikely(x) (__builtin_constant_p(x) ? !!(x) : 
__branch_check__(x, 0))
 ^~~~
>> fs/afs/internal.h:769:6: note: in expansion of macro 'unlikely'
 if (unlikely(afs_debug & AFS_DEBUG_KDEBUG)) \
 ^~~~
   fs/afs/main.c:68:2: note: in expansion of macro '_debug'
 _debug("AFS UUID: %08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
 ^~
>> include/linux/compiler.h:117:18: error: invalid use of undefined type 
>> 'struct uuid_v1'
   static struct ftrace_branch_data  \
 ^
   include/linux/compiler.h:139:58: note: in expansion of macro 
'__branch_check__'
#  define unlikely(x) (__builtin_constant_p(x) ? !!(x) : 

Re: [PATCH 1/2] afs: Move UUID struct to linux/uuid.h

[kbuild test robot]

Hi David,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.10-rc3 next-20170113]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/David-Howells/afs-Move-UUID-struct-to-linux-uuid-h/20170113-134212
config: x86_64-randconfig-b0-01131757 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

Note: the 
linux-review/David-Howells/afs-Move-UUID-struct-to-linux-uuid-h/20170113-134212 
HEAD c4259bc3968ea592af3a1599099327eb67e6019d builds fine.
  It only hurts bisectibility.

All error/warnings (new ones prefixed by >>):

   fs/afs/main.c: In function 'afs_get_client_UUID':
   fs/afs/main.c:48:2: error: invalid use of undefined type 'struct uuid_v1'
 ret = afs_get_MAC_address(afs_uuid.node, sizeof(afs_uuid.node));
 ^~~
   fs/afs/main.c:48:2: error: invalid use of undefined type 'struct uuid_v1'
   fs/afs/main.c:55:14: error: 'UUID_TO_UNIX_TIME' undeclared (first use in 
this function)
 uuidtime += UUID_TO_UNIX_TIME;
 ^
   fs/afs/main.c:55:14: note: each undeclared identifier is reported only once 
for each function it appears in
   fs/afs/main.c:56:2: error: invalid use of undefined type 'struct uuid_v1'
 afs_uuid.time_low = htonl(uuidtime);
 ^~~~
   fs/afs/main.c:57:2: error: invalid use of undefined type 'struct uuid_v1'
 afs_uuid.time_mid = htons(uuidtime >> 32);
 ^~~~
   fs/afs/main.c:58:28: error: 'UUID_TIMEHI_MASK' undeclared (first use in this 
function)
 hi_v = (uuidtime >> 48) & UUID_TIMEHI_MASK;
   ^~~~
   fs/afs/main.c:59:10: error: 'UUID_VERSION_TIME' undeclared (first use in 
this function)
 hi_v |= UUID_VERSION_TIME;
 ^
   fs/afs/main.c:60:2: error: invalid use of undefined type 'struct uuid_v1'
 afs_uuid.time_hi_and_version = htons(hi_v);
 ^~~~
   fs/afs/main.c:63:2: error: invalid use of undefined type 'struct uuid_v1'
 afs_uuid.clock_seq_low = clockseq;
 ^~~~
   fs/afs/main.c:64:2: error: invalid use of undefined type 'struct uuid_v1'
 afs_uuid.clock_seq_hi_and_reserved =
 ^~~~
   fs/afs/main.c:65:21: error: 'UUID_CLOCKHI_MASK' undeclared (first use in 
this function)
  (clockseq >> 8) & UUID_CLOCKHI_MASK;
^
   fs/afs/main.c:66:2: error: invalid use of undefined type 'struct uuid_v1'
 afs_uuid.clock_seq_hi_and_reserved |= UUID_VARIANT_STD;
 ^~~~
   fs/afs/main.c:66:40: error: 'UUID_VARIANT_STD' undeclared (first use in this 
function)
 afs_uuid.clock_seq_hi_and_reserved |= UUID_VARIANT_STD;
   ^~~~
   In file included from include/uapi/linux/stddef.h:1:0,
from include/linux/stddef.h:4,
from include/uapi/linux/posix_types.h:4,
from include/uapi/linux/types.h:13,
from include/linux/types.h:5,
from include/linux/list.h:4,
from include/linux/module.h:9,
from fs/afs/main.c:12:
>> include/linux/compiler.h:117:18: error: invalid use of undefined type 
>> 'struct uuid_v1'
   static struct ftrace_branch_data  \
 ^
   include/linux/compiler.h:139:58: note: in expansion of macro 
'__branch_check__'
#  define unlikely(x) (__builtin_constant_p(x) ? !!(x) : 
__branch_check__(x, 0))
 ^~~~
>> fs/afs/internal.h:769:6: note: in expansion of macro 'unlikely'
 if (unlikely(afs_debug & AFS_DEBUG_KDEBUG)) \
 ^~~~
   fs/afs/main.c:68:2: note: in expansion of macro '_debug'
 _debug("AFS UUID: %08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
 ^~
>> include/linux/compiler.h:117:18: error: invalid use of undefined type 
>> 'struct uuid_v1'
   static struct ftrace_branch_data  \
 ^
   include/linux/compiler.h:139:58: note: in expansion of macro 
'__branch_check__'
#  define unlikely(x) (__builtin_constant_p(x) ? !!(x) : 
__branch_check__(x, 0))
 ^~~~
>> fs/afs/internal.h:769:6: note: in expansion of macro 'unlikely'
 if (unlikely(afs_debug & AFS_DEBUG_KDEBUG)) \
 ^~~~
   fs/afs/main.c:68:2: note: in expansion of macro '_debug'
 _debug("AFS UUID: %08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
 ^~
>> include/linux/compiler.h:117:18: error: invalid use of undefined type 
>> 'struct uuid_v1'
   static struct ftrace_branch_data  \
 ^
   include/linux/compiler.h:139:58: note: in expansion of macro 
'__branch_check__'
#  define unlikely(x) (__builtin_constant_p(x) ? !!(x) : 

Re: [PATCH 1/2] afs: Move UUID struct to linux/uuid.h

[David Howells]

kbuild test robot  wrote:

>fs/afs/cmservice.c: In function 'afs_deliver_cb_init_call_back_state3':
> >> fs/afs/cmservice.c:365:34: error: invalid application of 'sizeof' to 
> >> incomplete type 'struct uuid_v1'
>   call->request = kmalloc(sizeof(struct uuid_v1), GFP_KERNEL);
>  ^~

Ah...  Missing #include of linux/uuid.h.  Under some circumstances it is
included indirectly.

David

Re: [PATCH 1/2] afs: Move UUID struct to linux/uuid.h

[David Howells]

kbuild test robot  wrote:

>fs/afs/cmservice.c: In function 'afs_deliver_cb_init_call_back_state3':
> >> fs/afs/cmservice.c:365:34: error: invalid application of 'sizeof' to 
> >> incomplete type 'struct uuid_v1'
>   call->request = kmalloc(sizeof(struct uuid_v1), GFP_KERNEL);
>  ^~

Ah...  Missing #include of linux/uuid.h.  Under some circumstances it is
included indirectly.

David

Re: [PATCH 1/2] afs: Move UUID struct to linux/uuid.h

[kbuild test robot]

Hi David,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.10-rc3 next-20170113]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/David-Howells/afs-Move-UUID-struct-to-linux-uuid-h/20170113-134212
config: x86_64-randconfig-s4-01131622 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   fs/afs/cmservice.c: In function 'afs_deliver_cb_init_call_back_state3':
>> fs/afs/cmservice.c:365:34: error: invalid application of 'sizeof' to 
>> incomplete type 'struct uuid_v1'
  call->request = kmalloc(sizeof(struct uuid_v1), GFP_KERNEL);
 ^~
>> fs/afs/cmservice.c:371:4: error: dereferencing pointer to incomplete type 
>> 'struct uuid_v1'
  r->time_low   = b[0];
   ^~
   fs/afs/cmservice.c: In function 'SRXAFSCB_ProbeUuid':
   fs/afs/cmservice.c:449:33: error: invalid application of 'sizeof' to 
incomplete type 'struct uuid_v1'
 if (memcmp(r, _uuid, sizeof(afs_uuid)) == 0)
^
   fs/afs/cmservice.c: In function 'afs_deliver_cb_probe_uuid':
   fs/afs/cmservice.c:489:34: error: invalid application of 'sizeof' to 
incomplete type 'struct uuid_v1'
  call->request = kmalloc(sizeof(struct uuid_v1), GFP_KERNEL);
 ^~
   fs/afs/cmservice.c: In function 'SRXAFSCB_TellMeAboutYourself':
>> fs/afs/cmservice.c:557:2: error: invalid use of undefined type 'struct 
>> uuid_v1'
 reply.ia.uuid[0] = afs_uuid.time_low;
 ^
   fs/afs/cmservice.c:558:2: error: invalid use of undefined type 'struct 
uuid_v1'
 reply.ia.uuid[1] = htonl(ntohl(afs_uuid.time_mid));
 ^
   fs/afs/cmservice.c:559:2: error: invalid use of undefined type 'struct 
uuid_v1'
 reply.ia.uuid[2] = htonl(ntohl(afs_uuid.time_hi_and_version));
 ^
   fs/afs/cmservice.c:560:2: error: invalid use of undefined type 'struct 
uuid_v1'
 reply.ia.uuid[3] = htonl((s8) afs_uuid.clock_seq_hi_and_reserved);
 ^
   fs/afs/cmservice.c:561:2: error: invalid use of undefined type 'struct 
uuid_v1'
 reply.ia.uuid[4] = htonl((s8) afs_uuid.clock_seq_low);
 ^
   fs/afs/cmservice.c:563:3: error: invalid use of undefined type 'struct 
uuid_v1'
  reply.ia.uuid[loop + 5] = htonl((s8) afs_uuid.node[loop]);
  ^
--
   fs/afs/main.c: In function 'afs_get_client_UUID':
>> fs/afs/main.c:48:2: error: invalid use of undefined type 'struct uuid_v1'
 ret = afs_get_MAC_address(afs_uuid.node, sizeof(afs_uuid.node));
 ^~~
>> fs/afs/main.c:48:2: error: invalid use of undefined type 'struct uuid_v1'
>> fs/afs/main.c:55:14: error: 'UUID_TO_UNIX_TIME' undeclared (first use in 
>> this function)
 uuidtime += UUID_TO_UNIX_TIME;
 ^
   fs/afs/main.c:55:14: note: each undeclared identifier is reported only once 
for each function it appears in
   fs/afs/main.c:56:2: error: invalid use of undefined type 'struct uuid_v1'
 afs_uuid.time_low = htonl(uuidtime);
 ^~~~
   fs/afs/main.c:57:2: error: invalid use of undefined type 'struct uuid_v1'
 afs_uuid.time_mid = htons(uuidtime >> 32);
 ^~~~
>> fs/afs/main.c:58:28: error: 'UUID_TIMEHI_MASK' undeclared (first use in this 
>> function)
 hi_v = (uuidtime >> 48) & UUID_TIMEHI_MASK;
   ^~~~
>> fs/afs/main.c:59:10: error: 'UUID_VERSION_TIME' undeclared (first use in 
>> this function)
 hi_v |= UUID_VERSION_TIME;
 ^
   fs/afs/main.c:60:2: error: invalid use of undefined type 'struct uuid_v1'
 afs_uuid.time_hi_and_version = htons(hi_v);
 ^~~~
   fs/afs/main.c:63:2: error: invalid use of undefined type 'struct uuid_v1'
 afs_uuid.clock_seq_low = clockseq;
 ^~~~
   fs/afs/main.c:64:2: error: invalid use of undefined type 'struct uuid_v1'
 afs_uuid.clock_seq_hi_and_reserved =
 ^~~~
>> fs/afs/main.c:65:21: error: 'UUID_CLOCKHI_MASK' undeclared (first use in 
>> this function)
  (clockseq >> 8) & UUID_CLOCKHI_MASK;
^
   fs/afs/main.c:66:2: error: invalid use of undefined type 'struct uuid_v1'
 afs_uuid.clock_seq_hi_and_reserved |= UUID_VARIANT_STD;
 ^~~~
>> fs/afs/main.c:66:40: error: 'UUID_VARIANT_STD' undeclared (first use in this 
>> function)
 afs_uuid.clock_seq_hi_and_reserved |= UUID_VARIANT_STD;
   ^~~~
   fs/afs/main.c:68:2: error: invalid use of undefined type 'struct uuid_v1'
 _debug("AFS UUID: %08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
 ^~
   fs/afs/main.c:68:2: error: invalid use of undefined type 'struct uuid_v1'
   fs/afs/main.c:68:2: error: invalid use of undefined type 'struct uuid_v1'
   

Re: [PATCH 1/2] afs: Move UUID struct to linux/uuid.h

[kbuild test robot]

Hi David,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.10-rc3 next-20170113]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/David-Howells/afs-Move-UUID-struct-to-linux-uuid-h/20170113-134212
config: x86_64-randconfig-s4-01131622 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

   fs/afs/cmservice.c: In function 'afs_deliver_cb_init_call_back_state3':
>> fs/afs/cmservice.c:365:34: error: invalid application of 'sizeof' to 
>> incomplete type 'struct uuid_v1'
  call->request = kmalloc(sizeof(struct uuid_v1), GFP_KERNEL);
 ^~
>> fs/afs/cmservice.c:371:4: error: dereferencing pointer to incomplete type 
>> 'struct uuid_v1'
  r->time_low   = b[0];
   ^~
   fs/afs/cmservice.c: In function 'SRXAFSCB_ProbeUuid':
   fs/afs/cmservice.c:449:33: error: invalid application of 'sizeof' to 
incomplete type 'struct uuid_v1'
 if (memcmp(r, _uuid, sizeof(afs_uuid)) == 0)
^
   fs/afs/cmservice.c: In function 'afs_deliver_cb_probe_uuid':
   fs/afs/cmservice.c:489:34: error: invalid application of 'sizeof' to 
incomplete type 'struct uuid_v1'
  call->request = kmalloc(sizeof(struct uuid_v1), GFP_KERNEL);
 ^~
   fs/afs/cmservice.c: In function 'SRXAFSCB_TellMeAboutYourself':
>> fs/afs/cmservice.c:557:2: error: invalid use of undefined type 'struct 
>> uuid_v1'
 reply.ia.uuid[0] = afs_uuid.time_low;
 ^
   fs/afs/cmservice.c:558:2: error: invalid use of undefined type 'struct 
uuid_v1'
 reply.ia.uuid[1] = htonl(ntohl(afs_uuid.time_mid));
 ^
   fs/afs/cmservice.c:559:2: error: invalid use of undefined type 'struct 
uuid_v1'
 reply.ia.uuid[2] = htonl(ntohl(afs_uuid.time_hi_and_version));
 ^
   fs/afs/cmservice.c:560:2: error: invalid use of undefined type 'struct 
uuid_v1'
 reply.ia.uuid[3] = htonl((s8) afs_uuid.clock_seq_hi_and_reserved);
 ^
   fs/afs/cmservice.c:561:2: error: invalid use of undefined type 'struct 
uuid_v1'
 reply.ia.uuid[4] = htonl((s8) afs_uuid.clock_seq_low);
 ^
   fs/afs/cmservice.c:563:3: error: invalid use of undefined type 'struct 
uuid_v1'
  reply.ia.uuid[loop + 5] = htonl((s8) afs_uuid.node[loop]);
  ^
--
   fs/afs/main.c: In function 'afs_get_client_UUID':
>> fs/afs/main.c:48:2: error: invalid use of undefined type 'struct uuid_v1'
 ret = afs_get_MAC_address(afs_uuid.node, sizeof(afs_uuid.node));
 ^~~
>> fs/afs/main.c:48:2: error: invalid use of undefined type 'struct uuid_v1'
>> fs/afs/main.c:55:14: error: 'UUID_TO_UNIX_TIME' undeclared (first use in 
>> this function)
 uuidtime += UUID_TO_UNIX_TIME;
 ^
   fs/afs/main.c:55:14: note: each undeclared identifier is reported only once 
for each function it appears in
   fs/afs/main.c:56:2: error: invalid use of undefined type 'struct uuid_v1'
 afs_uuid.time_low = htonl(uuidtime);
 ^~~~
   fs/afs/main.c:57:2: error: invalid use of undefined type 'struct uuid_v1'
 afs_uuid.time_mid = htons(uuidtime >> 32);
 ^~~~
>> fs/afs/main.c:58:28: error: 'UUID_TIMEHI_MASK' undeclared (first use in this 
>> function)
 hi_v = (uuidtime >> 48) & UUID_TIMEHI_MASK;
   ^~~~
>> fs/afs/main.c:59:10: error: 'UUID_VERSION_TIME' undeclared (first use in 
>> this function)
 hi_v |= UUID_VERSION_TIME;
 ^
   fs/afs/main.c:60:2: error: invalid use of undefined type 'struct uuid_v1'
 afs_uuid.time_hi_and_version = htons(hi_v);
 ^~~~
   fs/afs/main.c:63:2: error: invalid use of undefined type 'struct uuid_v1'
 afs_uuid.clock_seq_low = clockseq;
 ^~~~
   fs/afs/main.c:64:2: error: invalid use of undefined type 'struct uuid_v1'
 afs_uuid.clock_seq_hi_and_reserved =
 ^~~~
>> fs/afs/main.c:65:21: error: 'UUID_CLOCKHI_MASK' undeclared (first use in 
>> this function)
  (clockseq >> 8) & UUID_CLOCKHI_MASK;
^
   fs/afs/main.c:66:2: error: invalid use of undefined type 'struct uuid_v1'
 afs_uuid.clock_seq_hi_and_reserved |= UUID_VARIANT_STD;
 ^~~~
>> fs/afs/main.c:66:40: error: 'UUID_VARIANT_STD' undeclared (first use in this 
>> function)
 afs_uuid.clock_seq_hi_and_reserved |= UUID_VARIANT_STD;
   ^~~~
   fs/afs/main.c:68:2: error: invalid use of undefined type 'struct uuid_v1'
 _debug("AFS UUID: %08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
 ^~
   fs/afs/main.c:68:2: error: invalid use of undefined type 'struct uuid_v1'
   fs/afs/main.c:68:2: error: invalid use of undefined type 'struct uuid_v1'
   

Re: [PATCH 1/2] afs: Move UUID struct to linux/uuid.h

[David Howells]

Arnd Bergmann  wrote:

> Yes, looks good to me.

Can I put that down as a Reviewed-by?

David

Re: [PATCH 1/2] afs: Move UUID struct to linux/uuid.h

[David Howells]

Arnd Bergmann  wrote:

> Yes, looks good to me.

Can I put that down as a Reviewed-by?

David

Re: [PATCH 1/2] afs: Move UUID struct to linux/uuid.h

[Arnd Bergmann]

On Thu, Jan 12, 2017 at 5:14 PM, David Howells  wrote:
>> > reply.ia.uuid[3] = htonl((s8) afs_uuid.clock_seq_hi_and_reserved);
>> > reply.ia.uuid[4] = htonl((s8) afs_uuid.clock_seq_low);
>> > for (loop = 0; loop < 6; loop++)
>>
>> Shouldn't this be ntohs() instead of ntohl(), like this:
>>
>>reply.ia.uuid[1] = htonl(ntohl(afs_uuid.time_mid));
>>reply.ia.uuid[2] = htonl(ntohl(afs_uuid.time_hi_and_version));
>
> I think you forgot to change ntohl() to ntohs() in that - and you're right.
>
> Okay, how about the attached?

Yes, looks good to me.

  Arnd

Re: [PATCH 1/2] afs: Move UUID struct to linux/uuid.h

[Arnd Bergmann]

On Thu, Jan 12, 2017 at 5:14 PM, David Howells  wrote:
>> > reply.ia.uuid[3] = htonl((s8) afs_uuid.clock_seq_hi_and_reserved);
>> > reply.ia.uuid[4] = htonl((s8) afs_uuid.clock_seq_low);
>> > for (loop = 0; loop < 6; loop++)
>>
>> Shouldn't this be ntohs() instead of ntohl(), like this:
>>
>>reply.ia.uuid[1] = htonl(ntohl(afs_uuid.time_mid));
>>reply.ia.uuid[2] = htonl(ntohl(afs_uuid.time_hi_and_version));
>
> I think you forgot to change ntohl() to ntohs() in that - and you're right.
>
> Okay, how about the attached?

Yes, looks good to me.

  Arnd

Re: [PATCH 1/2] afs: Move UUID struct to linux/uuid.h

[David Howells]

Arnd Bergmann  wrote:

> > -   reply.ia.uuid[0] = htonl(afs_uuid.time_low);
> > -   reply.ia.uuid[1] = htonl(afs_uuid.time_mid);
> > -   reply.ia.uuid[2] = htonl(afs_uuid.time_hi_and_version);
> > +   reply.ia.uuid[0] = afs_uuid.time_low;
> > +   reply.ia.uuid[1] = htonl(ntohl(afs_uuid.time_mid));
> > +   reply.ia.uuid[2] = htonl(ntohl(afs_uuid.time_hi_and_version));
> > 
> > reply.ia.uuid[3] = htonl((s8) afs_uuid.clock_seq_hi_and_reserved);
> > reply.ia.uuid[4] = htonl((s8) afs_uuid.clock_seq_low);
> > for (loop = 0; loop < 6; loop++)
> 
> Shouldn't this be ntohs() instead of ntohl(), like this:
> 
>reply.ia.uuid[1] = htonl(ntohl(afs_uuid.time_mid));
>reply.ia.uuid[2] = htonl(ntohl(afs_uuid.time_hi_and_version));

I think you forgot to change ntohl() to ntohs() in that - and you're right.

Okay, how about the attached?

David
---
commit 459327d2968663ad3c5377d84c357e8f0b5fcd83
Author: David Howells 
Date:   Thu Jan 12 11:32:10 2017 +

afs: Move UUID struct to linux/uuid.h

Move the afs_uuid struct to linux/uuid.h, rename it to uuid_v1 and change
the u16/u32 fields to __be16/__be32 instead so that the structure can be
cast to a 16-octet network-order buffer.

Signed-off-by: David Howells 

diff --git a/fs/afs/cmservice.c b/fs/afs/cmservice.c
index e349a3316303..2edbdcbf6432 100644
--- a/fs/afs/cmservice.c
+++ b/fs/afs/cmservice.c
@@ -351,7 +351,7 @@ static int afs_deliver_cb_init_call_back_state3(struct 
afs_call *call)
 {
struct sockaddr_rxrpc srx;
struct afs_server *server;
-   struct afs_uuid *r;
+   struct uuid_v1 *r;
unsigned loop;
__be32 *b;
int ret;
@@ -381,15 +381,15 @@ static int afs_deliver_cb_init_call_back_state3(struct 
afs_call *call)
}
 
_debug("unmarshall UUID");
-   call->request = kmalloc(sizeof(struct afs_uuid), GFP_KERNEL);
+   call->request = kmalloc(sizeof(struct uuid_v1), GFP_KERNEL);
if (!call->request)
return -ENOMEM;
 
b = call->buffer;
r = call->request;
-   r->time_low = ntohl(b[0]);
-   r->time_mid = ntohl(b[1]);
-   r->time_hi_and_version  = ntohl(b[2]);
+   r->time_low = b[0];
+   r->time_mid = htons(ntohl(b[1]));
+   r->time_hi_and_version  = htons(ntohl(b[2]));
r->clock_seq_hi_and_reserved= ntohl(b[3]);
r->clock_seq_low= ntohl(b[4]);
 
@@ -454,7 +454,7 @@ static int afs_deliver_cb_probe(struct afs_call *call)
 static void SRXAFSCB_ProbeUuid(struct work_struct *work)
 {
struct afs_call *call = container_of(work, struct afs_call, work);
-   struct afs_uuid *r = call->request;
+   struct uuid_v1 *r = call->request;
 
struct {
__be32  match;
@@ -477,7 +477,7 @@ static void SRXAFSCB_ProbeUuid(struct work_struct *work)
  */
 static int afs_deliver_cb_probe_uuid(struct afs_call *call)
 {
-   struct afs_uuid *r;
+   struct uuid_v1 *r;
unsigned loop;
__be32 *b;
int ret;
@@ -503,15 +503,15 @@ static int afs_deliver_cb_probe_uuid(struct afs_call 
*call)
}
 
_debug("unmarshall UUID");
-   call->request = kmalloc(sizeof(struct afs_uuid), GFP_KERNEL);
+   call->request = kmalloc(sizeof(struct uuid_v1), GFP_KERNEL);
if (!call->request)
return -ENOMEM;
 
b = call->buffer;
r = call->request;
-   r->time_low = ntohl(b[0]);
-   r->time_mid = ntohl(b[1]);
-   r->time_hi_and_version  = ntohl(b[2]);
+   r->time_low = b[0];
+   r->time_mid = htons(ntohl(b[1]));
+   r->time_hi_and_version  = htons(ntohl(b[2]));
r->clock_seq_hi_and_reserved= ntohl(b[3]);
r->clock_seq_low= ntohl(b[4]);
 
@@ -569,9 +569,9 @@ static void SRXAFSCB_TellMeAboutYourself(struct work_struct 
*work)
memset(, 0, sizeof(reply));
reply.ia.nifs = htonl(nifs);
 
-   reply.ia.uuid[0] = htonl(afs_uuid.time_low);
-   reply.ia.uuid[1] = htonl(afs_uuid.time_mid);
-   reply.ia.uuid[2] = htonl(afs_uuid.time_hi_and_version);
+   reply.ia.uuid[0] = afs_uuid.time_low;
+   reply.ia.uuid[1] = htonl(ntohs(afs_uuid.time_mid));
+   reply.ia.uuid[2] = htonl(ntohs(afs_uuid.time_hi_and_version));
reply.ia.uuid[3] = htonl((s8) afs_uuid.clock_seq_hi_and_reserved);
reply.ia.uuid[4] = htonl((s8) afs_uuid.clock_seq_low);
for (loop = 0; 

Re: [PATCH 1/2] afs: Move UUID struct to linux/uuid.h

[David Howells]

Arnd Bergmann  wrote:

> > -   reply.ia.uuid[0] = htonl(afs_uuid.time_low);
> > -   reply.ia.uuid[1] = htonl(afs_uuid.time_mid);
> > -   reply.ia.uuid[2] = htonl(afs_uuid.time_hi_and_version);
> > +   reply.ia.uuid[0] = afs_uuid.time_low;
> > +   reply.ia.uuid[1] = htonl(ntohl(afs_uuid.time_mid));
> > +   reply.ia.uuid[2] = htonl(ntohl(afs_uuid.time_hi_and_version));
> > 
> > reply.ia.uuid[3] = htonl((s8) afs_uuid.clock_seq_hi_and_reserved);
> > reply.ia.uuid[4] = htonl((s8) afs_uuid.clock_seq_low);
> > for (loop = 0; loop < 6; loop++)
> 
> Shouldn't this be ntohs() instead of ntohl(), like this:
> 
>reply.ia.uuid[1] = htonl(ntohl(afs_uuid.time_mid));
>reply.ia.uuid[2] = htonl(ntohl(afs_uuid.time_hi_and_version));

I think you forgot to change ntohl() to ntohs() in that - and you're right.

Okay, how about the attached?

David
---
commit 459327d2968663ad3c5377d84c357e8f0b5fcd83
Author: David Howells 
Date:   Thu Jan 12 11:32:10 2017 +

afs: Move UUID struct to linux/uuid.h

Move the afs_uuid struct to linux/uuid.h, rename it to uuid_v1 and change
the u16/u32 fields to __be16/__be32 instead so that the structure can be
cast to a 16-octet network-order buffer.

Signed-off-by: David Howells 

diff --git a/fs/afs/cmservice.c b/fs/afs/cmservice.c
index e349a3316303..2edbdcbf6432 100644
--- a/fs/afs/cmservice.c
+++ b/fs/afs/cmservice.c
@@ -351,7 +351,7 @@ static int afs_deliver_cb_init_call_back_state3(struct 
afs_call *call)
 {
struct sockaddr_rxrpc srx;
struct afs_server *server;
-   struct afs_uuid *r;
+   struct uuid_v1 *r;
unsigned loop;
__be32 *b;
int ret;
@@ -381,15 +381,15 @@ static int afs_deliver_cb_init_call_back_state3(struct 
afs_call *call)
}
 
_debug("unmarshall UUID");
-   call->request = kmalloc(sizeof(struct afs_uuid), GFP_KERNEL);
+   call->request = kmalloc(sizeof(struct uuid_v1), GFP_KERNEL);
if (!call->request)
return -ENOMEM;
 
b = call->buffer;
r = call->request;
-   r->time_low = ntohl(b[0]);
-   r->time_mid = ntohl(b[1]);
-   r->time_hi_and_version  = ntohl(b[2]);
+   r->time_low = b[0];
+   r->time_mid = htons(ntohl(b[1]));
+   r->time_hi_and_version  = htons(ntohl(b[2]));
r->clock_seq_hi_and_reserved= ntohl(b[3]);
r->clock_seq_low= ntohl(b[4]);
 
@@ -454,7 +454,7 @@ static int afs_deliver_cb_probe(struct afs_call *call)
 static void SRXAFSCB_ProbeUuid(struct work_struct *work)
 {
struct afs_call *call = container_of(work, struct afs_call, work);
-   struct afs_uuid *r = call->request;
+   struct uuid_v1 *r = call->request;
 
struct {
__be32  match;
@@ -477,7 +477,7 @@ static void SRXAFSCB_ProbeUuid(struct work_struct *work)
  */
 static int afs_deliver_cb_probe_uuid(struct afs_call *call)
 {
-   struct afs_uuid *r;
+   struct uuid_v1 *r;
unsigned loop;
__be32 *b;
int ret;
@@ -503,15 +503,15 @@ static int afs_deliver_cb_probe_uuid(struct afs_call 
*call)
}
 
_debug("unmarshall UUID");
-   call->request = kmalloc(sizeof(struct afs_uuid), GFP_KERNEL);
+   call->request = kmalloc(sizeof(struct uuid_v1), GFP_KERNEL);
if (!call->request)
return -ENOMEM;
 
b = call->buffer;
r = call->request;
-   r->time_low = ntohl(b[0]);
-   r->time_mid = ntohl(b[1]);
-   r->time_hi_and_version  = ntohl(b[2]);
+   r->time_low = b[0];
+   r->time_mid = htons(ntohl(b[1]));
+   r->time_hi_and_version  = htons(ntohl(b[2]));
r->clock_seq_hi_and_reserved= ntohl(b[3]);
r->clock_seq_low= ntohl(b[4]);
 
@@ -569,9 +569,9 @@ static void SRXAFSCB_TellMeAboutYourself(struct work_struct 
*work)
memset(, 0, sizeof(reply));
reply.ia.nifs = htonl(nifs);
 
-   reply.ia.uuid[0] = htonl(afs_uuid.time_low);
-   reply.ia.uuid[1] = htonl(afs_uuid.time_mid);
-   reply.ia.uuid[2] = htonl(afs_uuid.time_hi_and_version);
+   reply.ia.uuid[0] = afs_uuid.time_low;
+   reply.ia.uuid[1] = htonl(ntohs(afs_uuid.time_mid));
+   reply.ia.uuid[2] = htonl(ntohs(afs_uuid.time_hi_and_version));
reply.ia.uuid[3] = htonl((s8) afs_uuid.clock_seq_hi_and_reserved);
reply.ia.uuid[4] = htonl((s8) afs_uuid.clock_seq_low);
for (loop = 0; loop < 6; loop++)
diff --git a/fs/afs/internal.h 

Re: [PATCH 1/2] afs: Move UUID struct to linux/uuid.h

[Arnd Bergmann]

On Thursday, January 12, 2017 2:12:56 PM CET David Howells wrote:
> Arnd Bergmann  wrote:
> 
> > Looks good to me, but I wonder if this part:
> > 
> > r = call->request;
> > -   r->time_low = ntohl(b[0]);
> > -   r->time_mid = ntohl(b[1]);
> > -   r->time_hi_and_version  = ntohl(b[2]);
> > +   r->time_low = b[0];
> > +   r->time_mid = htons(ntohl(b[1]));
> > +   r->time_hi_and_version  = htons(ntohl(b[2]));
> > r->clock_seq_hi_and_reserved= ntohl(b[3]);
> > r->clock_seq_low= ntohl(b[4]);
> >  
> > should be considered a bugfix and split out into a
> > separate patch.
> 
> I changed the definitions in the struct from u16/u32 to __be16/__be32 so it's
> not a bugfix.

Ok.

> > From what I understand about the mess in UUID formats, the time fields can
> > either be big-endian (as defined) or little-endian (for all things
> > Microsoft),
> 
> RFC 4122 specified that the multi-octet fields are stored MSB-first.
> 
> > and you are changing the representation from CPU-specific to big-endian,
> > which makes it different for x86 and most ARM at least.
> 
> In-kernel, not in the protocol.

Ok, I assumed that the uuid was later sent out over the wire again
in the in-memory format, but you are right, it does get sent out in
the AFS specific format as a series of 32-bit big-endian values
rather than the RFC4122 format.
 
> The problem is that you can't do what you put in your suggested patch and just
> copy the UUID produced by the generate_random_uuid() over the afs_uuid struct
> since that puts the version in the wrong place.

Got it. One more thing then:

> @@ -569,9 +569,9 @@ static void SRXAFSCB_TellMeAboutYourself(struct
> work_struct *work)> 
> memset(, 0, sizeof(reply));
> reply.ia.nifs = htonl(nifs);
> 
> -   reply.ia.uuid[0] = htonl(afs_uuid.time_low);
> -   reply.ia.uuid[1] = htonl(afs_uuid.time_mid);
> -   reply.ia.uuid[2] = htonl(afs_uuid.time_hi_and_version);
> +   reply.ia.uuid[0] = afs_uuid.time_low;
> +   reply.ia.uuid[1] = htonl(ntohl(afs_uuid.time_mid));
> +   reply.ia.uuid[2] = htonl(ntohl(afs_uuid.time_hi_and_version));
> 
> reply.ia.uuid[3] = htonl((s8) afs_uuid.clock_seq_hi_and_reserved);
> reply.ia.uuid[4] = htonl((s8) afs_uuid.clock_seq_low);
> for (loop = 0; loop < 6; loop++)

Shouldn't this be ntohs() instead of ntohl(), like this:

   reply.ia.uuid[1] = htonl(ntohl(afs_uuid.time_mid));
   reply.ia.uuid[2] = htonl(ntohl(afs_uuid.time_hi_and_version));

My head is spinning a little from all the byteswapping, but it
looks to me like the data here ends up in the wrong half of the
on-wire data. Can you double-check this?

Arnd

Re: [PATCH 1/2] afs: Move UUID struct to linux/uuid.h

[Arnd Bergmann]

On Thursday, January 12, 2017 2:12:56 PM CET David Howells wrote:
> Arnd Bergmann  wrote:
> 
> > Looks good to me, but I wonder if this part:
> > 
> > r = call->request;
> > -   r->time_low = ntohl(b[0]);
> > -   r->time_mid = ntohl(b[1]);
> > -   r->time_hi_and_version  = ntohl(b[2]);
> > +   r->time_low = b[0];
> > +   r->time_mid = htons(ntohl(b[1]));
> > +   r->time_hi_and_version  = htons(ntohl(b[2]));
> > r->clock_seq_hi_and_reserved= ntohl(b[3]);
> > r->clock_seq_low= ntohl(b[4]);
> >  
> > should be considered a bugfix and split out into a
> > separate patch.
> 
> I changed the definitions in the struct from u16/u32 to __be16/__be32 so it's
> not a bugfix.

Ok.

> > From what I understand about the mess in UUID formats, the time fields can
> > either be big-endian (as defined) or little-endian (for all things
> > Microsoft),
> 
> RFC 4122 specified that the multi-octet fields are stored MSB-first.
> 
> > and you are changing the representation from CPU-specific to big-endian,
> > which makes it different for x86 and most ARM at least.
> 
> In-kernel, not in the protocol.

Ok, I assumed that the uuid was later sent out over the wire again
in the in-memory format, but you are right, it does get sent out in
the AFS specific format as a series of 32-bit big-endian values
rather than the RFC4122 format.
 
> The problem is that you can't do what you put in your suggested patch and just
> copy the UUID produced by the generate_random_uuid() over the afs_uuid struct
> since that puts the version in the wrong place.

Got it. One more thing then:

> @@ -569,9 +569,9 @@ static void SRXAFSCB_TellMeAboutYourself(struct
> work_struct *work)> 
> memset(, 0, sizeof(reply));
> reply.ia.nifs = htonl(nifs);
> 
> -   reply.ia.uuid[0] = htonl(afs_uuid.time_low);
> -   reply.ia.uuid[1] = htonl(afs_uuid.time_mid);
> -   reply.ia.uuid[2] = htonl(afs_uuid.time_hi_and_version);
> +   reply.ia.uuid[0] = afs_uuid.time_low;
> +   reply.ia.uuid[1] = htonl(ntohl(afs_uuid.time_mid));
> +   reply.ia.uuid[2] = htonl(ntohl(afs_uuid.time_hi_and_version));
> 
> reply.ia.uuid[3] = htonl((s8) afs_uuid.clock_seq_hi_and_reserved);
> reply.ia.uuid[4] = htonl((s8) afs_uuid.clock_seq_low);
> for (loop = 0; loop < 6; loop++)

Shouldn't this be ntohs() instead of ntohl(), like this:

   reply.ia.uuid[1] = htonl(ntohl(afs_uuid.time_mid));
   reply.ia.uuid[2] = htonl(ntohl(afs_uuid.time_hi_and_version));

My head is spinning a little from all the byteswapping, but it
looks to me like the data here ends up in the wrong half of the
on-wire data. Can you double-check this?

Arnd

Re: [PATCH 1/2] afs: Move UUID struct to linux/uuid.h

[David Howells]

Arnd Bergmann  wrote:

> Looks good to me, but I wonder if this part:
> 
> r = call->request;
> -   r->time_low = ntohl(b[0]);
> -   r->time_mid = ntohl(b[1]);
> -   r->time_hi_and_version  = ntohl(b[2]);
> +   r->time_low = b[0];
> +   r->time_mid = htons(ntohl(b[1]));
> +   r->time_hi_and_version  = htons(ntohl(b[2]));
> r->clock_seq_hi_and_reserved= ntohl(b[3]);
> r->clock_seq_low= ntohl(b[4]);
>  
> should be considered a bugfix and split out into a
> separate patch.

I changed the definitions in the struct from u16/u32 to __be16/__be32 so it's
not a bugfix.

For some reason, rather than specifying UUIDs as just a 16-octet field, the
AFS protocol breaks the UUID down into pieces and converts them into 32-bit
fields (apparently signed in some places:-/).

> From what I understand about the mess in UUID formats, the time fields can
> either be big-endian (as defined) or little-endian (for all things
> Microsoft),

RFC 4122 specified that the multi-octet fields are stored MSB-first.

> and you are changing the representation from CPU-specific to big-endian,
> which makes it different for x86 and most ARM at least.

In-kernel, not in the protocol.

The problem is that you can't do what you put in your suggested patch and just
copy the UUID produced by the generate_random_uuid() over the afs_uuid struct
since that puts the version in the wrong place.

David

Re: [PATCH 1/2] afs: Move UUID struct to linux/uuid.h

[David Howells]

Arnd Bergmann  wrote:

> Looks good to me, but I wonder if this part:
> 
> r = call->request;
> -   r->time_low = ntohl(b[0]);
> -   r->time_mid = ntohl(b[1]);
> -   r->time_hi_and_version  = ntohl(b[2]);
> +   r->time_low = b[0];
> +   r->time_mid = htons(ntohl(b[1]));
> +   r->time_hi_and_version  = htons(ntohl(b[2]));
> r->clock_seq_hi_and_reserved= ntohl(b[3]);
> r->clock_seq_low= ntohl(b[4]);
>  
> should be considered a bugfix and split out into a
> separate patch.

I changed the definitions in the struct from u16/u32 to __be16/__be32 so it's
not a bugfix.

For some reason, rather than specifying UUIDs as just a 16-octet field, the
AFS protocol breaks the UUID down into pieces and converts them into 32-bit
fields (apparently signed in some places:-/).

> From what I understand about the mess in UUID formats, the time fields can
> either be big-endian (as defined) or little-endian (for all things
> Microsoft),

RFC 4122 specified that the multi-octet fields are stored MSB-first.

> and you are changing the representation from CPU-specific to big-endian,
> which makes it different for x86 and most ARM at least.

In-kernel, not in the protocol.

The problem is that you can't do what you put in your suggested patch and just
copy the UUID produced by the generate_random_uuid() over the afs_uuid struct
since that puts the version in the wrong place.

David

Re: [PATCH 1/2] afs: Move UUID struct to linux/uuid.h

[Arnd Bergmann]

On Thursday, January 12, 2017 11:56:34 AM CET David Howells wrote:
> Move the afs_uuid struct to linux/uuid.h and rename it to uuid_v1.
> 
> Signed-off-by: David Howells 
> 

Looks good to me, but I wonder if this part:

r = call->request;
-   r->time_low = ntohl(b[0]);
-   r->time_mid = ntohl(b[1]);
-   r->time_hi_and_version  = ntohl(b[2]);
+   r->time_low = b[0];
+   r->time_mid = htons(ntohl(b[1]));
+   r->time_hi_and_version  = htons(ntohl(b[2]));
r->clock_seq_hi_and_reserved= ntohl(b[3]);
r->clock_seq_low= ntohl(b[4]);
 
should be considered a bugfix and split out into a
separate patch. From what I understand about the mess in UUID
formats, the time fields can either be big-endian (as defined)
or little-endian (for all things Microsoft), and you are changing
the representation from CPU-specific to big-endian, which makes
it different for x86 and most ARM at least.

Arnd

Re: [PATCH 1/2] afs: Move UUID struct to linux/uuid.h

[Arnd Bergmann]

On Thursday, January 12, 2017 11:56:34 AM CET David Howells wrote:
> Move the afs_uuid struct to linux/uuid.h and rename it to uuid_v1.
> 
> Signed-off-by: David Howells 
> 

Looks good to me, but I wonder if this part:

r = call->request;
-   r->time_low = ntohl(b[0]);
-   r->time_mid = ntohl(b[1]);
-   r->time_hi_and_version  = ntohl(b[2]);
+   r->time_low = b[0];
+   r->time_mid = htons(ntohl(b[1]));
+   r->time_hi_and_version  = htons(ntohl(b[2]));
r->clock_seq_hi_and_reserved= ntohl(b[3]);
r->clock_seq_low= ntohl(b[4]);
 
should be considered a bugfix and split out into a
separate patch. From what I understand about the mess in UUID
formats, the time fields can either be big-endian (as defined)
or little-endian (for all things Microsoft), and you are changing
the representation from CPU-specific to big-endian, which makes
it different for x86 and most ARM at least.

Arnd