All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nathan Chancellor <natechancellor@gmail.com>
To: Bart Van Assche <bvanassche@acm.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>,
	"James E.J. Bottomley" <jejb@linux.vnet.ibm.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	linux-scsi@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>
Subject: Re: -Wswitch Clang warnings in drivers/scsi
Date: Thu, 18 Oct 2018 23:51:04 -0700	[thread overview]
Message-ID: <20181019065104.GA27170@flashbox> (raw)
In-Reply-To: <1539024429.64374.40.camel@acm.org>

On Mon, Oct 08, 2018 at 11:47:09AM -0700, Bart Van Assche wrote:
> On Thu, 2018-10-04 at 23:57 -0700, Nathan Chancellor wrote:
> > Regardless of how the overflow is handled within the switch statement,
> > the overflow is also happening when passing in these values to the ioctl,
> > right? I mean these case values are defined in the uapi files so that
> > userspace can easily pass them in to the ioctl, meaning those values are
> > being passed in as a signed integer and I would assume subsequently
> > overflowing unless I'm just missing something here.
> 
> From the user space header <sys/ioctl.h>:
> 
> extern int ioctl (int __fd, unsigned long int __request, ...) __THROW;
> 
> From the kernel header <linux/fs.h>:
> 
> 	long (*unlocked_ioctl) (struct file *, unsigned int, unsigned long);
> 	long (*compat_ioctl) (struct file *, unsigned int, unsigned long);
> 
> Why has the second argument been declared as "unsigned long" in the glibc
> headers and as "unsigned int" in the kernel headers? That's not clear to me.
> 
> Bart.
> 

Hi Bart,

Sorry it took me so long to reply, somehow this email got lost in my
inbox...

Unfortuntely, I am unsure why there is that discrepency between the
headers. I tried to do some research but I didn't come up with much.

However, I did test changing the type of ioctl/compat_ioctl's cmd
parameter to 'unsigned int' and came up with the following diff (rather
large so sharing via a gist instead of pasting here):

https://gist.github.com/nathanchance/8febc92735f4228574cb0464520f0f6f

I'll obviously draft up a proper commit message before formally sending
but I can address any major concerns before that happens. I checked
every single ioctl for a negative value and there aren't any so I think
this change makes sense to fix this warning.

Cheers,
Nathan

  reply	other threads:[~2018-10-19  6:51 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-04 18:30 -Wswitch Clang warnings in drivers/scsi Nathan Chancellor
2018-10-04 18:34 ` Bart Van Assche
2018-10-04 18:45   ` Nathan Chancellor
2018-10-04 21:16     ` Nick Desaulniers
2018-10-05  6:57       ` Nathan Chancellor
2018-10-08 18:12         ` Nick Desaulniers
2018-10-08 18:47         ` Bart Van Assche
2018-10-08 18:47           ` Bart Van Assche
2018-10-19  6:51           ` Nathan Chancellor [this message]
2018-10-19 13:55             ` Bart Van Assche

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181019065104.GA27170@flashbox \
    --to=natechancellor@gmail.com \
    --cc=bvanassche@acm.org \
    --cc=jejb@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=ndesaulniers@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.