In regard of commit "gudev: Use strtoul to parse unsigned 64-bit

In regard of commit "gudev: Use strtoul to parse unsigned 64-bit

[Rafał Mużyło]

Perhaps I'm simply misreading something, but won't going from strtoll to
strtoul break things for 32bit case ?

I do understand, that 32bit is becoming less and less supported every
day (given i.e. "daemon.c:1351:33: warning: cast to pointer from integer
of different size" warning during udisks 1.0.4 build - bug 24983), but
should it be broken on purpose ?

Re: In regard of commit "gudev: Use strtoul to parse unsigned 64-bit integers"

[David Zeuthen]

On Mon, Oct 31, 2011 at 4:13 PM, Rafał Mużyło <galtgendo@gmail.com> wrote:
> Perhaps I'm simply misreading something, but won't going from strtoll to
> strtoul break things for 32bit case ?

What do you think it would break? Or are you perhaps suggesting we
should be using strtoull() instead? Please be specific.


     David

Re: In regard of commit "gudev: Use strtoul to parse unsigned 64-bit integers"

[David Zeuthen]

[-- Attachment #1: Type: text/plain, Size: 675 bytes --]

Hi,

I looked more into this and we should probably just use
g_ascii_strtoull() since that function always returns a guint64 no
matter whether you are on 32- or 64-bit. The attached patch does this.
Kay: please apply.

Thanks,
David

On Mon, Oct 31, 2011 at 4:32 PM, David Zeuthen <zeuthen@gmail.com> wrote:
> On Mon, Oct 31, 2011 at 4:13 PM, Rafał Mużyło <galtgendo@gmail.com> wrote:
>> Perhaps I'm simply misreading something, but won't going from strtoll to
>> strtoul break things for 32bit case ?
>
> What do you think it would break? Or are you perhaps suggesting we
> should be using strtoull() instead? Please be specific.
>
>
>     David
>

[-- Attachment #2: 0001-gudev-Use-g_ascii_strtoull-instead-of-strtoul.patch --]
[-- Type: text/x-patch, Size: 1770 bytes --]

From 2a64773ac80432ca70aa747b55cfaadf3f8c55cb Mon Sep 17 00:00:00 2001
From: David Zeuthen <davidz@redhat.com>
Date: Mon, 31 Oct 2011 16:38:14 -0400
Subject: [PATCH] gudev: Use g_ascii_strtoull() instead of strtoul()

This ensures that we get the same behavior on both 32- and
64-bit. Pointed out on the mailing list:

 http://permalink.gmane.org/gmane.linux.hotplug.devel/17145

Signed-off-by: David Zeuthen <davidz@redhat.com>
---
 extras/gudev/gudevdevice.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/extras/gudev/gudevdevice.c b/extras/gudev/gudevdevice.c
index e77b34b..38d602c 100644
--- a/extras/gudev/gudevdevice.c
+++ b/extras/gudev/gudevdevice.c
@@ -538,7 +538,7 @@ out:
  * @key: Name of property.
  *
  * Look up the value for @key on @device and convert it to an unsigned
- * 64-bit integer using strtoul().
+ * 64-bit integer using g_ascii_strtoull().
  *
  * Returns: The value  for @key or 0 if @key doesn't  exist or isn't a
  * #guint64.
@@ -558,7 +558,7 @@ g_udev_device_get_property_as_uint64 (GUdevDevice  *device,
   if (s == NULL)
     goto out;
 
-  result = strtoul (s, NULL, 0);
+  result = g_ascii_strtoull (s, NULL, 0);
 out:
   return result;
 }
@@ -756,7 +756,7 @@ out:
  * @name: Name of the sysfs attribute.
  *
  * Look up the sysfs attribute with @name on @device and convert it to an unsigned
- * 64-bit integer using strtoul().
+ * 64-bit integer using g_ascii_strtoull().
  *
  * Returns: The value of the sysfs attribute or 0 if there is no such
  * attribute.
@@ -776,7 +776,7 @@ g_udev_device_get_sysfs_attr_as_uint64 (GUdevDevice  *device,
   if (s == NULL)
     goto out;
 
-  result = strtoul (s, NULL, 0);
+  result = g_ascii_strtoull (s, NULL, 0);
 out:
   return result;
 }
-- 
1.7.6.4

Re: In regard of commit "gudev: Use strtoul to parse unsigned 64-bit

[Rafał Mużyło]

On Mon, Oct 31, 2011 at 04:32:07PM -0400, David Zeuthen wrote:
> On Mon, Oct 31, 2011 at 4:13 PM, Rafał Mużyło <galtgendo@gmail.com> wrote:
> > Perhaps I'm simply misreading something, but won't going from strtoll to
> > strtoul break things for 32bit case ?
> 
> What do you think it would break? Or are you perhaps suggesting we
> should be using strtoull() instead? Please be specific.
>
Well, given on what happens on strtoul overflow according to the
manpage...

But, as you're the main author of udisks, would you mind looking at the
bug I've mentioned ?
Perhaps it doesn't matter there, but it would be good to do something
about the warnings.

Re: In regard of commit "gudev: Use strtoul to parse unsigned 64-bit integers"

[David Zeuthen]

On Mon, Oct 31, 2011 at 5:04 PM, Rafał Mużyło <galtgendo@gmail.com> wrote:
> On Mon, Oct 31, 2011 at 04:32:07PM -0400, David Zeuthen wrote:
>> On Mon, Oct 31, 2011 at 4:13 PM, Rafał Mużyło <galtgendo@gmail.com> wrote:
>> > Perhaps I'm simply misreading something, but won't going from strtoll to
>> > strtoul break things for 32bit case ?
>>
>> What do you think it would break? Or are you perhaps suggesting we
>> should be using strtoull() instead? Please be specific.
>>
> Well, given on what happens on strtoul overflow according to the
> manpage...

Well, the problem was simply that I just missed an 'l' in the initial
patch and that mistake breaks 32-bit as you tried to point out. The
patch I just sent fixes the problem. Sigh, this is what happens when
people try to make their function names as short as possible... never
a good idea.

    David

Re: In regard of commit "gudev: Use strtoul to parse unsigned 64-bit integers"

[Kay Sievers]

On Mon, Oct 31, 2011 at 21:44, David Zeuthen <zeuthen@gmail.com> wrote:
> I looked more into this and we should probably just use
> g_ascii_strtoull() since that function always returns a guint64 no
> matter whether you are on 32- or 64-bit. The attached patch does this.
> Kay: please apply.

Done.

Thanks,
Kay