2014-11-10 21:09:42

by Kevin Cernekee

[permalink] [raw]
Subject: [PATCH 1/2] [CIFS] Fix signed/unsigned pointer warning

Commit 2ae83bf93882d1 ("[CIFS] Fix setting time before epoch (negative
time values)") changed "u64 t" to "s64 t", which makes do_div() complain
about a pointer signedness mismatch:

CC fs/cifs/netmisc.o
In file included from ./arch/mips/include/asm/div64.h:12:0,
from include/linux/kernel.h:124,
from include/linux/list.h:8,
from include/linux/wait.h:6,
from include/linux/net.h:23,
from fs/cifs/netmisc.c:25:
fs/cifs/netmisc.c: In function ‘cifs_NTtimeToUnix’:
include/asm-generic/div64.h:43:28: warning: comparison of distinct pointer types lacks a cast [enabled by default]
(void)(((typeof((n)) *)0) == ((uint64_t *)0)); \
^
fs/cifs/netmisc.c:941:22: note: in expansion of macro ‘do_div’
ts.tv_nsec = (long)do_div(t, 10000000) * 100;

Introduce a temporary "u64 abs_t" variable to fix this.

Signed-off-by: Kevin Cernekee <[email protected]>
---
fs/cifs/netmisc.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)


Note: this is compile-tested only (on a 32-bit build).


diff --git a/fs/cifs/netmisc.c b/fs/cifs/netmisc.c
index b333ff6..abae6dd 100644
--- a/fs/cifs/netmisc.c
+++ b/fs/cifs/netmisc.c
@@ -926,6 +926,7 @@ cifs_NTtimeToUnix(__le64 ntutc)

/* Subtract the NTFS time offset, then convert to 1s intervals. */
s64 t = le64_to_cpu(ntutc) - NTFS_TIME_OFFSET;
+ u64 abs_t;

/*
* Unfortunately can not use normal 64 bit division on 32 bit arch, but
@@ -933,13 +934,14 @@ cifs_NTtimeToUnix(__le64 ntutc)
* to special case them
*/
if (t < 0) {
- t = -t;
- ts.tv_nsec = (long)(do_div(t, 10000000) * 100);
+ abs_t = -t;
+ ts.tv_nsec = (long)(do_div(abs_t, 10000000) * 100);
ts.tv_nsec = -ts.tv_nsec;
- ts.tv_sec = -t;
+ ts.tv_sec = -abs_t;
} else {
- ts.tv_nsec = (long)do_div(t, 10000000) * 100;
- ts.tv_sec = t;
+ abs_t = t;
+ ts.tv_nsec = (long)do_div(abs_t, 10000000) * 100;
+ ts.tv_sec = abs_t;
}

return ts;
--
2.1.1


2014-11-10 21:09:52

by Kevin Cernekee

[permalink] [raw]
Subject: [PATCH 2/2] [CIFS] Update MAINTAINERS entry

Update link to point to Steve's current tree on git.samba.org. Remove
link to the old patchwork instance which shows no new patches since 2010.

Signed-off-by: Kevin Cernekee <[email protected]>
---
MAINTAINERS | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index b36a4f0..4d8154d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2519,8 +2519,7 @@ M: Steve French <[email protected]>
L: [email protected]
L: [email protected] (moderated for non-subscribers)
W: http://linux-cifs.samba.org/
-Q: http://patchwork.ozlabs.org/project/linux-cifs-client/list/
-T: git git://git.kernel.org/pub/scm/linux/kernel/git/sfrench/cifs-2.6.git
+T: git git://git.samba.org/sfrench/cifs-2.6.git
S: Supported
F: Documentation/filesystems/cifs/
F: fs/cifs/
--
2.1.1

2014-11-11 02:58:26

by Steve French

[permalink] [raw]
Subject: Re: [PATCH 2/2] [CIFS] Update MAINTAINERS entry

merged into cifs-2.6.git

(and reviewing your other patch)

On Mon, Nov 10, 2014 at 3:09 PM, Kevin Cernekee <[email protected]> wrote:
> Update link to point to Steve's current tree on git.samba.org. Remove
> link to the old patchwork instance which shows no new patches since 2010.
>
> Signed-off-by: Kevin Cernekee <[email protected]>
> ---
> MAINTAINERS | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b36a4f0..4d8154d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2519,8 +2519,7 @@ M: Steve French <[email protected]>
> L: [email protected]
> L: [email protected] (moderated for non-subscribers)
> W: http://linux-cifs.samba.org/
> -Q: http://patchwork.ozlabs.org/project/linux-cifs-client/list/
> -T: git git://git.kernel.org/pub/scm/linux/kernel/git/sfrench/cifs-2.6.git
> +T: git git://git.samba.org/sfrench/cifs-2.6.git
> S: Supported
> F: Documentation/filesystems/cifs/
> F: fs/cifs/
> --
> 2.1.1
>



--
Thanks,

Steve

2014-12-12 17:20:18

by Kevin Cernekee

[permalink] [raw]
Subject: Re: [PATCH 1/2] [CIFS] Fix signed/unsigned pointer warning

On Mon, Nov 10, 2014 at 1:09 PM, Kevin Cernekee <[email protected]> wrote:
> Commit 2ae83bf93882d1 ("[CIFS] Fix setting time before epoch (negative
> time values)") changed "u64 t" to "s64 t", which makes do_div() complain
> about a pointer signedness mismatch:
>
> CC fs/cifs/netmisc.o
> In file included from ./arch/mips/include/asm/div64.h:12:0,
> from include/linux/kernel.h:124,
> from include/linux/list.h:8,
> from include/linux/wait.h:6,
> from include/linux/net.h:23,
> from fs/cifs/netmisc.c:25:
> fs/cifs/netmisc.c: In function ‘cifs_NTtimeToUnix’:
> include/asm-generic/div64.h:43:28: warning: comparison of distinct pointer types lacks a cast [enabled by default]
> (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \
> ^
> fs/cifs/netmisc.c:941:22: note: in expansion of macro ‘do_div’
> ts.tv_nsec = (long)do_div(t, 10000000) * 100;
>
> Introduce a temporary "u64 abs_t" variable to fix this.
>
> Signed-off-by: Kevin Cernekee <[email protected]>
> ---
> fs/cifs/netmisc.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
>
> Note: this is compile-tested only (on a 32-bit build).
>
>
> diff --git a/fs/cifs/netmisc.c b/fs/cifs/netmisc.c
> index b333ff6..abae6dd 100644
> --- a/fs/cifs/netmisc.c
> +++ b/fs/cifs/netmisc.c
> @@ -926,6 +926,7 @@ cifs_NTtimeToUnix(__le64 ntutc)
>
> /* Subtract the NTFS time offset, then convert to 1s intervals. */
> s64 t = le64_to_cpu(ntutc) - NTFS_TIME_OFFSET;
> + u64 abs_t;
>
> /*
> * Unfortunately can not use normal 64 bit division on 32 bit arch, but
> @@ -933,13 +934,14 @@ cifs_NTtimeToUnix(__le64 ntutc)
> * to special case them
> */
> if (t < 0) {
> - t = -t;
> - ts.tv_nsec = (long)(do_div(t, 10000000) * 100);
> + abs_t = -t;
> + ts.tv_nsec = (long)(do_div(abs_t, 10000000) * 100);
> ts.tv_nsec = -ts.tv_nsec;
> - ts.tv_sec = -t;
> + ts.tv_sec = -abs_t;
> } else {
> - ts.tv_nsec = (long)do_div(t, 10000000) * 100;
> - ts.tv_sec = t;
> + abs_t = t;
> + ts.tv_nsec = (long)do_div(abs_t, 10000000) * 100;
> + ts.tv_sec = abs_t;
> }
>
> return ts;
> --
> 2.1.1
>

Hi guys,

I am still seeing this warning on Linus' head of tree. Do you think
this is something we can pull in for 3.19?

Thanks!

2014-12-14 05:20:29

by Steve French

[permalink] [raw]
Subject: Re: [PATCH 1/2] [CIFS] Fix signed/unsigned pointer warning

Probably harmless patch - but I didn't notice the warning on x86
kernel build (building on Fedora 21, gcc 4.9.2)

On Fri, Dec 12, 2014 at 11:19 AM, Kevin Cernekee <[email protected]> wrote:
> On Mon, Nov 10, 2014 at 1:09 PM, Kevin Cernekee <[email protected]> wrote:
>> Commit 2ae83bf93882d1 ("[CIFS] Fix setting time before epoch (negative
>> time values)") changed "u64 t" to "s64 t", which makes do_div() complain
>> about a pointer signedness mismatch:
>>
>> CC fs/cifs/netmisc.o
>> In file included from ./arch/mips/include/asm/div64.h:12:0,
>> from include/linux/kernel.h:124,
>> from include/linux/list.h:8,
>> from include/linux/wait.h:6,
>> from include/linux/net.h:23,
>> from fs/cifs/netmisc.c:25:
>> fs/cifs/netmisc.c: In function ‘cifs_NTtimeToUnix’:
>> include/asm-generic/div64.h:43:28: warning: comparison of distinct pointer types lacks a cast [enabled by default]
>> (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \
>> ^
>> fs/cifs/netmisc.c:941:22: note: in expansion of macro ‘do_div’
>> ts.tv_nsec = (long)do_div(t, 10000000) * 100;
>>
>> Introduce a temporary "u64 abs_t" variable to fix this.
>>
>> Signed-off-by: Kevin Cernekee <[email protected]>
>> ---
>> fs/cifs/netmisc.c | 12 +++++++-----
>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>
>>
>> Note: this is compile-tested only (on a 32-bit build).
>>
>>
>> diff --git a/fs/cifs/netmisc.c b/fs/cifs/netmisc.c
>> index b333ff6..abae6dd 100644
>> --- a/fs/cifs/netmisc.c
>> +++ b/fs/cifs/netmisc.c
>> @@ -926,6 +926,7 @@ cifs_NTtimeToUnix(__le64 ntutc)
>>
>> /* Subtract the NTFS time offset, then convert to 1s intervals. */
>> s64 t = le64_to_cpu(ntutc) - NTFS_TIME_OFFSET;
>> + u64 abs_t;
>>
>> /*
>> * Unfortunately can not use normal 64 bit division on 32 bit arch, but
>> @@ -933,13 +934,14 @@ cifs_NTtimeToUnix(__le64 ntutc)
>> * to special case them
>> */
>> if (t < 0) {
>> - t = -t;
>> - ts.tv_nsec = (long)(do_div(t, 10000000) * 100);
>> + abs_t = -t;
>> + ts.tv_nsec = (long)(do_div(abs_t, 10000000) * 100);
>> ts.tv_nsec = -ts.tv_nsec;
>> - ts.tv_sec = -t;
>> + ts.tv_sec = -abs_t;
>> } else {
>> - ts.tv_nsec = (long)do_div(t, 10000000) * 100;
>> - ts.tv_sec = t;
>> + abs_t = t;
>> + ts.tv_nsec = (long)do_div(abs_t, 10000000) * 100;
>> + ts.tv_sec = abs_t;
>> }
>>
>> return ts;
>> --
>> 2.1.1
>>
>
> Hi guys,
>
> I am still seeing this warning on Linus' head of tree. Do you think
> this is something we can pull in for 3.19?
>
> Thanks!



--
Thanks,

Steve

2014-12-14 05:29:33

by Kevin Cernekee

[permalink] [raw]
Subject: Re: [PATCH 1/2] [CIFS] Fix signed/unsigned pointer warning

On Sat, Dec 13, 2014 at 9:20 PM, Steve French <[email protected]> wrote:
> Probably harmless patch - but I didn't notice the warning on x86
> kernel build (building on Fedora 21, gcc 4.9.2)

Did you test x86 32-bit or 64-bit? In the generic do_div()
implementation, only the former case has an explicit type check.

FWIW I'm on 32-bit bit MIPS.

2014-12-15 00:29:01

by Steve French

[permalink] [raw]
Subject: Re: [PATCH 1/2] [CIFS] Fix signed/unsigned pointer warning

merged into cifs-2.6.git

On Sat, Dec 13, 2014 at 11:29 PM, Kevin Cernekee <[email protected]> wrote:
> On Sat, Dec 13, 2014 at 9:20 PM, Steve French <[email protected]> wrote:
>> Probably harmless patch - but I didn't notice the warning on x86
>> kernel build (building on Fedora 21, gcc 4.9.2)
>
> Did you test x86 32-bit or 64-bit? In the generic do_div()

I built current mainline with 64 bit disabled. No warnings on this
before or after the patch.

> implementation, only the former case has an explicit type check.
>
> FWIW I'm on 32-bit bit MIPS.



--
Thanks,

Steve

2014-12-15 00:54:42

by Kevin Cernekee

[permalink] [raw]
Subject: Re: [PATCH 1/2] [CIFS] Fix signed/unsigned pointer warning

On Sun, Dec 14, 2014 at 4:28 PM, Steve French <[email protected]> wrote:
> On Sat, Dec 13, 2014 at 11:29 PM, Kevin Cernekee <[email protected]> wrote:
>> On Sat, Dec 13, 2014 at 9:20 PM, Steve French <[email protected]> wrote:
>>> Probably harmless patch - but I didn't notice the warning on x86
>>> kernel build (building on Fedora 21, gcc 4.9.2)
>>
>> Did you test x86 32-bit or 64-bit? In the generic do_div()
>
> I built current mainline with 64 bit disabled. No warnings on this
> before or after the patch.

Ah, right, it only shows up on 32-bit architectures that use the
generic do_div implementation. To see the warning on x86, you'd need
to do something like this:

$ git --no-pager diff
diff --git a/arch/x86/include/asm/div64.h b/arch/x86/include/asm/div64.h
index ced283a..f693002 100644
--- a/arch/x86/include/asm/div64.h
+++ b/arch/x86/include/asm/div64.h
@@ -1,7 +1,7 @@
#ifndef _ASM_X86_DIV64_H
#define _ASM_X86_DIV64_H

-#ifdef CONFIG_X86_32
+#if 0//def CONFIG_X86_32

#include <linux/types.h>
#include <linux/log2.h>
$ touch fs/cifs/netmisc.c
$ make ARCH=x86 vmlinux
CHK include/config/kernel.release
CHK include/generated/uapi/linux/version.h
CHK include/generated/utsrelease.h
CALL scripts/checksyscalls.sh
CHK include/generated/compile.h
CC fs/cifs/netmisc.o
fs/cifs/netmisc.c: In function ‘cifs_NTtimeToUnix’:
fs/cifs/netmisc.c:937:23: warning: comparison of distinct pointer
types lacks a cast [enabled by default]
fs/cifs/netmisc.c:941:22: warning: comparison of distinct pointer
types lacks a cast [enabled by default]
LD fs/cifs/cifs.o
LD fs/cifs/built-in.o
LD fs/built-in.o
LINK vmlinux
LD vmlinux.o
MODPOST vmlinux.o
GEN .version
CHK include/generated/compile.h
UPD include/generated/compile.h
CC init/version.o
LD init/built-in.o
KSYM .tmp_kallsyms1.o
KSYM .tmp_kallsyms2.o
LD vmlinux
SORTEX vmlinux
SYSMAP System.map
$


> merged into cifs-2.6.git

Thanks!