Skip to contentSkip to conversation

gpiolib: Switch to use compat_need_64bit_alignment_fixup() helper

PATCH Superseded Andy Shevchenko 2 participants 5 messages Oct 8, 2020 – Oct 12, 2020

Subsystems (2)
GPIO GPIO uapi

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Andy Shevchenko

author
#0 lore raw

PATCH v2 1/1 gpiolib: Switch to use compat_need_64bit_alignment_fixup() helper

The new compat_need_64bit_alignment_fixup() helper allows to avoid ugly ifdeffery in IOCTL compatible code. Use it in GPIO cdev code.

Depends-on: 527c412519eb ("compat: add a compat_need_64bit_alignment_fixup() helper")
Depends-on: cc7886d25bca ("compat: lift compat_s64 and compat_u64 to <asm-generic/compat.h>")

Signed-off-by Andy Shevchenko <[email protected]>

---
v2: removed lineevent_get_size() completely, fixed typo in commit message

 drivers/gpio/gpiolib-cdev.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)
drivers/gpio/gpiolib-cdev.c +8 -16
index e9faeaf65d14..192721f829a3 100644
@@ -1479,21 +1479,10 @@ static __poll_t lineevent_poll(struct file *file,
 	return events;
 }
 
-static ssize_t lineevent_get_size(void)
-{
-#if defined(CONFIG_X86_64) && !defined(CONFIG_UML)
-	/* i386 has no padding after 'id' */
-	if (in_ia32_syscall()) {
-		struct compat_gpioeevent_data {
-			compat_u64	timestamp;
-			u32		id;
-		};
-
-		return sizeof(struct compat_gpioeevent_data);
-	}
-#endif
-	return sizeof(struct gpioevent_data);
-}
+struct compat_gpioeevent_data {
+	compat_u64	timestamp;
+	u32		id;
+};
 
 static ssize_t lineevent_read(struct file *file,
 			      char __user *buf,

@@ -1515,7 +1504,10 @@ static ssize_t lineevent_read(struct file *file,
 	 * actual sizeof() and pass this as an argument to copy_to_user() to
 	 * drop unneeded bytes from the output.
 	 */
-	ge_size = lineevent_get_size();
+	if (compat_need_64bit_alignment_fixup())
+		ge_size = sizeof(struct compat_gpioeevent_data);
+	else
+		ge_size = sizeof(struct gpioevent_data);
 	if (count < ge_size)
 		return -EINVAL;
Signature · 1 line
2.28.0
reviewer
#1 lore raw
Quoted 5 lines · Andy Shevchenko

The new compat_need_64bit_alignment_fixup() helper allows to avoid ugly ifdeffery in IOCTL compatible code. Use it in GPIO cdev code.

Depends-on: 527c412519eb ("compat: add a compat_need_64bit_alignment_fixup() helper")
Depends-on: cc7886d25bca ("compat: lift compat_s64 and compat_u64 to <asm-generic/compat.h>")

Where can I find these?

Cheers,
Kent.

author
#2 lore raw
Quoted 8 lines · Kent Gibson
Quoted 5 lines · Andy Shevchenko

The new compat_need_64bit_alignment_fixup() helper allows to avoid ugly ifdeffery in IOCTL compatible code. Use it in GPIO cdev code.

Depends-on: 527c412519eb ("compat: add a compat_need_64bit_alignment_fixup() helper")
Depends-on: cc7886d25bca ("compat: lift compat_s64 and compat_u64 to <asm-generic/compat.h>")

Where can I find these?

Soon in Linux upstream, but currently in Al Viro's tree: https://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git/log/?h=work.quota-compat

Signature · 2 lines
With Best Regards,
Andy Shevchenko
reviewer Tested-by on v2 1/1
#3 lore raw
In reply to Andy Shevchenko
Quoted 3 lines · Andy Shevchenko

The new compat_need_64bit_alignment_fixup() helper allows to avoid ugly ifdeffery in IOCTL compatible code. Use it in GPIO cdev code.

I'd re-arrange that checkin comment to

"Use the new compat_need_64bit_alignment_fixup() helper to avoid ugly ifdeffery in IOCTL compatibility code."

but otherwise all good.

+1 on the deuglification, and it still works too.

Tested-by Kent Gibson <[email protected]>
Quoted 53 lines

Depends-on: 527c412519eb ("compat: add a compat_need_64bit_alignment_fixup() helper")
Depends-on: cc7886d25bca ("compat: lift compat_s64 and compat_u64 to <asm-generic/compat.h>")

Signed-off-by Andy Shevchenko <[email protected]>

---
v2: removed lineevent_get_size() completely, fixed typo in commit message

 drivers/gpio/gpiolib-cdev.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)
drivers/gpio/gpiolib-cdev.c +8 -16
index e9faeaf65d14..192721f829a3 100644
@@ -1479,21 +1479,10 @@ static __poll_t lineevent_poll(struct file *file,
 	return events;
 }
 
-static ssize_t lineevent_get_size(void)
-{
-#if defined(CONFIG_X86_64) && !defined(CONFIG_UML)
-	/* i386 has no padding after 'id' */
-	if (in_ia32_syscall()) {
-		struct compat_gpioeevent_data {
-			compat_u64	timestamp;
-			u32		id;
-		};
-
-		return sizeof(struct compat_gpioeevent_data);
-	}
-#endif
-	return sizeof(struct gpioevent_data);
-}
+struct compat_gpioeevent_data {
+	compat_u64	timestamp;
+	u32		id;
+};
 
 static ssize_t lineevent_read(struct file *file,
 			      char __user *buf,

@@ -1515,7 +1504,10 @@ static ssize_t lineevent_read(struct file *file,
 	 * actual sizeof() and pass this as an argument to copy_to_user() to
 	 * drop unneeded bytes from the output.
 	 */
-	ge_size = lineevent_get_size();
+	if (compat_need_64bit_alignment_fixup())
+		ge_size = sizeof(struct compat_gpioeevent_data);
+	else
+		ge_size = sizeof(struct gpioevent_data);
 	if (count < ge_size)
 		return -EINVAL;
Signature · 1 line
2.28.0
author
#4 lore raw
Quoted 8 lines · Kent Gibson
Quoted 2 lines · Andy Shevchenko

The new compat_need_64bit_alignment_fixup() helper allows to avoid ugly ifdeffery in IOCTL compatible code. Use it in GPIO cdev code.

I'd re-arrange that checkin comment to

"Use the new compat_need_64bit_alignment_fixup() helper to avoid ugly ifdeffery in IOCTL compatibility code."

OK!

Quoted 5 lines

but otherwise all good.

+1 on the deuglification, and it still works too.

Tested-by Kent Gibson <[email protected]>

Thanks.

Quoted 3 lines
Quoted 3 lines

Depends-on: 527c412519eb ("compat: add a compat_need_64bit_alignment_fixup() helper")

Depends-on: cc7886d25bca ("compat: lift compat_s64 and compat_u64 to <asm-generic/compat.h>")

I'll also drop this one, since former is dependent on latter.

Signature · 2 lines
With Best Regards,
Andy Shevchenko