Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1626675 > unrolled thread

Re: [PATCH] fs: compat: Remove warning from COMPATIBLE_IOCTL

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] fs: compat: Remove warning from COMPATIBLE_IOCTL Matthias Kaehlcke <mka@chromium.org> - 2017-04-19 20:20 +0200
    Re: [PATCH] fs: compat: Remove warning from COMPATIBLE_IOCTL Arnd Bergmann <arnd@arndb.de> - 2017-04-19 22:50 +0200
      Re: [PATCH] fs: compat: Remove warning from COMPATIBLE_IOCTL Matthias Kaehlcke <mka@chromium.org> - 2017-04-19 23:40 +0200
        Re: [PATCH] fs: compat: Remove warning from COMPATIBLE_IOCTL Arnd Bergmann <arnd@arndb.de> - 2017-04-20 10:50 +0200
          Re: [PATCH] fs: compat: Remove warning from COMPATIBLE_IOCTL Matthias Kaehlcke <mka@chromium.org> - 2017-04-20 22:00 +0200

#1626675 — Re: [PATCH] fs: compat: Remove warning from COMPATIBLE_IOCTL

El Tue, Apr 04, 2017 at 11:07:20AM -0700 Matthias Kaehlcke ha dit:

> From: Mark Charlebois <charlebm@gmail.com>
> 
> cmd in COMPATIBLE_IOCTL is always a u32, so cast it so there isn't a
> warning about an overflow in XFORM.
> 
> From: Mark Charlebois <charlebm@gmail.com>
> Signed-off-by: Mark Charlebois <charlebm@gmail.com>
> Signed-off-by: Behan Webster <behanw@converseincode.com>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> ---
> Resending https://patchwork.kernel.org/patch/4961631/
> 
>  fs/compat_ioctl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
> index 11d087b2b28e..6116d5275a3e 100644
> --- a/fs/compat_ioctl.c
> +++ b/fs/compat_ioctl.c
> @@ -833,7 +833,7 @@ static int compat_ioctl_preallocate(struct file *file,
>   */
>  #define XFORM(i) (((i) ^ ((i) << 27) ^ ((i) << 17)) & 0xffffffff)
>  
> -#define COMPATIBLE_IOCTL(cmd) XFORM(cmd),
> +#define COMPATIBLE_IOCTL(cmd) XFORM((u32)cmd),
>  /* ioctl should not be warned about even if it's not implemented.
>     Valid reasons to use this:
>     - It is implemented with ->compat_ioctl on some device, but programs

Ping, any feedback on this change?

Thanks

Matthias

[toc] | [next] | [standalone]

#1626789

On Wed, Apr 19, 2017 at 8:14 PM, Matthias Kaehlcke <mka@chromium.org> wrote:
> El Tue, Apr 04, 2017 at 11:07:20AM -0700 Matthias Kaehlcke ha dit:
>
>> From: Mark Charlebois <charlebm@gmail.com>
>>
>> cmd in COMPATIBLE_IOCTL is always a u32, so cast it so there isn't a
>> warning about an overflow in XFORM.
>>
>> From: Mark Charlebois <charlebm@gmail.com>
>> Signed-off-by: Mark Charlebois <charlebm@gmail.com>
>> Signed-off-by: Behan Webster <behanw@converseincode.com>
>> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
>> Acked-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>> Resending https://patchwork.kernel.org/patch/4961631/

The patch looks correct to me,

Acked-by: Arnd Bergmann <arnd@arndb.de>

>>  fs/compat_ioctl.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
>> index 11d087b2b28e..6116d5275a3e 100644
>> --- a/fs/compat_ioctl.c
>> +++ b/fs/compat_ioctl.c
>> @@ -833,7 +833,7 @@ static int compat_ioctl_preallocate(struct file *file,
>>   */
>>  #define XFORM(i) (((i) ^ ((i) << 27) ^ ((i) << 17)) & 0xffffffff)
>>
>> -#define COMPATIBLE_IOCTL(cmd) XFORM(cmd),
>> +#define COMPATIBLE_IOCTL(cmd) XFORM((u32)cmd),
>>  /* ioctl should not be warned about even if it's not implemented.
>>     Valid reasons to use this:
>>     - It is implemented with ->compat_ioctl on some device, but programs
>
> Ping, any feedback on this change?

One minor comment on the patch: when you address a warning in a patch,
it helps to put the compiler warning output into the changelog.

Aside from that, I see that you are upstreaming a number of clang
related patches. I actually have a longer series of clang patches that
I took from llvmlinux and hacked up to the point where I could
build ARM randconfig kernels without any warnings or errors.
If you are interested, I can separate the clang patches from my normal
randconfig build tree and upload the git tree for you to look at and
cherry-pick further patches.

      Arnd

[toc] | [prev] | [next] | [standalone]

#1626817

Hi Arnd,

El Wed, Apr 19, 2017 at 10:48:47PM +0200 Arnd Bergmann ha dit:

> On Wed, Apr 19, 2017 at 8:14 PM, Matthias Kaehlcke <mka@chromium.org> wrote:
> > El Tue, Apr 04, 2017 at 11:07:20AM -0700 Matthias Kaehlcke ha dit:
> >
> >> From: Mark Charlebois <charlebm@gmail.com>
> >>
> >> cmd in COMPATIBLE_IOCTL is always a u32, so cast it so there isn't a
> >> warning about an overflow in XFORM.
> >>
> >> From: Mark Charlebois <charlebm@gmail.com>
> >> Signed-off-by: Mark Charlebois <charlebm@gmail.com>
> >> Signed-off-by: Behan Webster <behanw@converseincode.com>
> >> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> >> Acked-by: Arnd Bergmann <arnd@arndb.de>
> >> ---
> >> Resending https://patchwork.kernel.org/patch/4961631/
> 
> The patch looks correct to me,
> 
> Acked-by: Arnd Bergmann <arnd@arndb.de>

Thanks for the review!

> >>  fs/compat_ioctl.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
> >> index 11d087b2b28e..6116d5275a3e 100644
> >> --- a/fs/compat_ioctl.c
> >> +++ b/fs/compat_ioctl.c
> >> @@ -833,7 +833,7 @@ static int compat_ioctl_preallocate(struct file *file,
> >>   */
> >>  #define XFORM(i) (((i) ^ ((i) << 27) ^ ((i) << 17)) & 0xffffffff)
> >>
> >> -#define COMPATIBLE_IOCTL(cmd) XFORM(cmd),
> >> +#define COMPATIBLE_IOCTL(cmd) XFORM((u32)cmd),
> >>  /* ioctl should not be warned about even if it's not implemented.
> >>     Valid reasons to use this:
> >>     - It is implemented with ->compat_ioctl on some device, but programs
> >
> > Ping, any feedback on this change?
> 
> One minor comment on the patch: when you address a warning in a patch,
> it helps to put the compiler warning output into the changelog.

Thanks, I added it initially and then wondered if it is too much
noise. Will take it into account in the future.

> Aside from that, I see that you are upstreaming a number of clang
> related patches. I actually have a longer series of clang patches that
> I took from llvmlinux and hacked up to the point where I could
> build ARM randconfig kernels without any warnings or errors.
> If you are interested, I can separate the clang patches from my normal
> randconfig build tree and upload the git tree for you to look at and
> cherry-pick further patches.

Sure, that would be interesting, though I won't promise to take up
everything :)

Cheers

Matthias

[toc] | [prev] | [next] | [standalone]

#1627212

On Wed, Apr 19, 2017 at 11:37 PM, Matthias Kaehlcke <mka@chromium.org> wrote:
> El Wed, Apr 19, 2017 at 10:48:47PM +0200 Arnd Bergmann ha dit:
>> On Wed, Apr 19, 2017 at 8:14 PM, Matthias Kaehlcke <mka@chromium.org> wrote:
>> > El Tue, Apr 04, 2017 at 11:07:20AM -0700 Matthias Kaehlcke ha dit:

>> Aside from that, I see that you are upstreaming a number of clang
>> related patches. I actually have a longer series of clang patches that
>> I took from llvmlinux and hacked up to the point where I could
>> build ARM randconfig kernels without any warnings or errors.
>> If you are interested, I can separate the clang patches from my normal
>> randconfig build tree and upload the git tree for you to look at and
>> cherry-pick further patches.
>
> Sure, that would be interesting, though I won't promise to take up
> everything :)

I see now that almost all the patches I did have been picked up in the
meantime, so I'm sure you already have most of the remaining contents
as they come from the old llvmlinux project. For reference, I've uploaded
my set to

git://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git
next-20170420+llvmlinux

This is rebased from the middle of a larger series, so there might be
problems I introduce. Just have a look to see what you can use.

      Arnd

[toc] | [prev] | [next] | [standalone]

#1627773

El Thu, Apr 20, 2017 at 10:45:47AM +0200 Arnd Bergmann ha dit:

> On Wed, Apr 19, 2017 at 11:37 PM, Matthias Kaehlcke <mka@chromium.org> wrote:
> > El Wed, Apr 19, 2017 at 10:48:47PM +0200 Arnd Bergmann ha dit:
> >> On Wed, Apr 19, 2017 at 8:14 PM, Matthias Kaehlcke <mka@chromium.org> wrote:
> >> > El Tue, Apr 04, 2017 at 11:07:20AM -0700 Matthias Kaehlcke ha dit:
> 
> >> Aside from that, I see that you are upstreaming a number of clang
> >> related patches. I actually have a longer series of clang patches that
> >> I took from llvmlinux and hacked up to the point where I could
> >> build ARM randconfig kernels without any warnings or errors.
> >> If you are interested, I can separate the clang patches from my normal
> >> randconfig build tree and upload the git tree for you to look at and
> >> cherry-pick further patches.
> >
> > Sure, that would be interesting, though I won't promise to take up
> > everything :)
> 
> I see now that almost all the patches I did have been picked up in the
> meantime, so I'm sure you already have most of the remaining contents
> as they come from the old llvmlinux project.

Yeah, I picked a subset of the llvmlinux patches from different
sources.

> For reference, I've uploaded my set to
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git
> next-20170420+llvmlinux
> 
> This is rebased from the middle of a larger series, so there might be
> problems I introduce. Just have a look to see what you can use.

Thanks!

Matthias

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web