| Message ID | 1429507996-28224-1-git-send-email-xuw2015@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On 4/20/15 12:33 AM, xuw2015@gmail.com wrote: > From: George Wang <xuw2015@gmail.com> > > PPC64 arch use such following IOC values " > \#define _IOC_NONE 1U > \#define _IOC_READ 2U > \#define _IOC_WRITE 4U > " comparing to the default IOC values " > \#define _IOC_NONE 0U > \#define _IOC_READ 2U > \#define _IOC_WRITE 1U" > > This means the value "_IOW*" will be negative when we store it in the int > variables. Such as the "BTRFS_IOC_QGROUP_CREATE", it will be "0x4010942e" on > X86_64, but "0x8010942e" on PPC64. > Notice that the IOC values are the "unsigned long" type, so we use the > "unsigned long" to store it, and this can insure the comparison between the > variable and BTRFS_IOC_* valid. Looks good - very strange that the manpage states that the interface takes an int. :( But - an "unsigned int" would be enough, right? I don't think it needs to be an unsigned long. *shrug* Thanks, -Eric > Signed-off-by: George Wang <xuw2015@gmail.com> > --- > cmds-quota.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/cmds-quota.c b/cmds-quota.c > index 89cc89c..f6a1cfa 100644 > --- a/cmds-quota.c > +++ b/cmds-quota.c > @@ -109,7 +109,7 @@ static int cmd_quota_rescan(int argc, char **argv) > int e; > char *path = NULL; > struct btrfs_ioctl_quota_rescan_args args; > - int ioctlnum = BTRFS_IOC_QUOTA_RESCAN; > + unsigned long ioctlnum = BTRFS_IOC_QUOTA_RESCAN; > DIR *dirstream = NULL; > int wait_for_completion = 0; > > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Thanks for review. On Mon, Apr 20, 2015 at 10:47 PM, Eric Sandeen <sandeen@redhat.com> wrote: > On 4/20/15 12:33 AM, xuw2015@gmail.com wrote: >> From: George Wang <xuw2015@gmail.com> <snip> >> This means the value "_IOW*" will be negative when we store it in the int >> variables. Such as the "BTRFS_IOC_QGROUP_CREATE", it will be "0x4010942e" on >> X86_64, but "0x8010942e" on PPC64. >> Notice that the IOC values are the "unsigned long" type, so we use the >> "unsigned long" to store it, and this can insure the comparison between the >> variable and BTRFS_IOC_* valid. > > Looks good - very strange that the manpage states that the interface takes > an int. :( I think maybe the manpage of ioctl is stale. But int can also work except for comparison. And fortunately the kernel interface is unsigned int, such as btrfs: 5217 long btrfs_ioctl(struct file *file, unsigned int 5218 cmd, unsigned long arg) > > But - an "unsigned int" would be enough, right? I don't think it needs > to be an unsigned long. *shrug* unsigned int will be OK. But the btrfs-progs is user space, so the ioctl I think it should be consistent with the glibc: /usr/include/sys/ioctl.h:extern int ioctl (int __fd, unsigned long int __request, ...) __THROW; unsigned long int is same with unsigned long. So maybe the unsigned long will be better? Thanks, George -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 21, 2015 at 10:26:23AM +0800, ?? wrote: > > On 4/20/15 12:33 AM, xuw2015@gmail.com wrote: > >> From: George Wang <xuw2015@gmail.com> > <snip> > >> This means the value "_IOW*" will be negative when we store it in the int > >> variables. Such as the "BTRFS_IOC_QGROUP_CREATE", it will be "0x4010942e" on > >> X86_64, but "0x8010942e" on PPC64. > >> Notice that the IOC values are the "unsigned long" type, so we use the > >> "unsigned long" to store it, and this can insure the comparison between the > >> variable and BTRFS_IOC_* valid. > > > > Looks good - very strange that the manpage states that the interface takes > > an int. :( > > I think maybe the manpage of ioctl is stale. But int can also work > except for comparison. > And fortunately the kernel interface is unsigned int, such as btrfs: > > 5217 long btrfs_ioctl(struct file *file, unsigned int > 5218 cmd, unsigned long arg) > > > > > But - an "unsigned int" would be enough, right? I don't think it needs > > to be an unsigned long. *shrug* > > unsigned int will be OK. But the btrfs-progs is user space, so the > ioctl I think it should be > consistent with the glibc: > /usr/include/sys/ioctl.h:extern int ioctl (int __fd, unsigned long int > __request, ...) __THROW; > unsigned long int is same with unsigned long. So maybe the unsigned > long will be better? I think we should stick to the glibc interface as it's the closest one, although kernel uses usigned int in the end. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday 20 Apr 2015 13:33:16 xuw2015@gmail.com wrote: > From: George Wang <xuw2015@gmail.com> > > PPC64 arch use such following IOC values " > \#define _IOC_NONE 1U > \#define _IOC_READ 2U > \#define _IOC_WRITE 4U > " comparing to the default IOC values " > \#define _IOC_NONE 0U > \#define _IOC_READ 2U > \#define _IOC_WRITE 1U" > > This means the value "_IOW*" will be negative when we store it in the int > variables. Such as the "BTRFS_IOC_QGROUP_CREATE", it will be "0x4010942e" on > X86_64, but "0x8010942e" on PPC64. > Notice that the IOC values are the "unsigned long" type, so we use the > "unsigned long" to store it, and this can insure the comparison between the > variable and BTRFS_IOC_* valid. > > Signed-off-by: George Wang <xuw2015@gmail.com> Tested-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
On Thu, Apr 23, 2015 at 10:43:29PM +0530, Chandan Rajendra wrote: > On Monday 20 Apr 2015 13:33:16 xuw2015@gmail.com wrote: > > From: George Wang <xuw2015@gmail.com> > > > > PPC64 arch use such following IOC values " > > \#define _IOC_NONE 1U > > \#define _IOC_READ 2U > > \#define _IOC_WRITE 4U > > " comparing to the default IOC values " > > \#define _IOC_NONE 0U > > \#define _IOC_READ 2U > > \#define _IOC_WRITE 1U" > > > > This means the value "_IOW*" will be negative when we store it in the int > > variables. Such as the "BTRFS_IOC_QGROUP_CREATE", it will be "0x4010942e" on > > X86_64, but "0x8010942e" on PPC64. > > Notice that the IOC values are the "unsigned long" type, so we use the > > "unsigned long" to store it, and this can insure the comparison between the > > variable and BTRFS_IOC_* valid. > > > > Signed-off-by: George Wang <xuw2015@gmail.com> > > Tested-by: Chandan Rajendra <chandan@linux.vnet.ibm.com> Thanks for testing, commit updated. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/cmds-quota.c b/cmds-quota.c index 89cc89c..f6a1cfa 100644 --- a/cmds-quota.c +++ b/cmds-quota.c @@ -109,7 +109,7 @@ static int cmd_quota_rescan(int argc, char **argv) int e; char *path = NULL; struct btrfs_ioctl_quota_rescan_args args; - int ioctlnum = BTRFS_IOC_QUOTA_RESCAN; + unsigned long ioctlnum = BTRFS_IOC_QUOTA_RESCAN; DIR *dirstream = NULL; int wait_for_completion = 0;