Patchwork [1/4] strtosz(): use unsigned char and switch to qemu_isspace()

login
register
mail settings
Submitter Jes Sorensen
Date Jan. 24, 2011, 3:33 p.m.
Message ID <1295883211-18288-2-git-send-email-Jes.Sorensen@redhat.com>
Download mbox | patch
Permalink /patch/80182/
State New
Headers show

Comments

Jes Sorensen - Jan. 24, 2011, 3:33 p.m.
From: Jes Sorensen <Jes.Sorensen@redhat.com>

isspace() behavior is undefined for signed char.

Bug pointed out by Eric Blake, thanks!

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 cutils.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)
Markus Armbruster - Jan. 24, 2011, 4:10 p.m.
Jes.Sorensen@redhat.com writes:

> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>
> isspace() behavior is undefined for signed char.
>
> Bug pointed out by Eric Blake, thanks!
>
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> ---
>  cutils.c |    7 ++++---
>  1 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/cutils.c b/cutils.c
> index 4d2e27c..a067cf4 100644
> --- a/cutils.c
> +++ b/cutils.c
> @@ -294,7 +294,8 @@ int fcntl_setfl(int fd, int flag)
>  int64_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
>  {
>      int64_t retval = -1;
> -    char *endptr, c, d;
> +    char *endptr;
> +    unsigned char c, d;
>      int mul_required = 0;
>      double val, mul, integral, fraction;
>  

I doubt this hunk is still needed.

> @@ -314,7 +315,7 @@ int64_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
>       */
>      c = *endptr;
>      d = c;
> -    if (isspace(c) || c == '\0' || c == ',') {
> +    if (qemu_isspace(c) || c == '\0' || c == ',') {
>          c = 0;
>          if (default_suffix) {
>              d = default_suffix;
> @@ -361,7 +362,7 @@ int64_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
>       */
>      if (c != 0) {
>          endptr++;
> -        if (!isspace(*endptr) && *endptr != ',' && *endptr != 0) {
> +        if (!qemu_isspace(*endptr) && *endptr != ',' && *endptr != 0) {
>              goto fail;
>          }
>      }
Jes Sorensen - Jan. 24, 2011, 4:11 p.m.
On 01/24/11 17:10, Markus Armbruster wrote:
> Jes.Sorensen@redhat.com writes:
> 
>> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>>
>> isspace() behavior is undefined for signed char.
>>
>> Bug pointed out by Eric Blake, thanks!
>>
>> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
>> ---
>>  cutils.c |    7 ++++---
>>  1 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/cutils.c b/cutils.c
>> index 4d2e27c..a067cf4 100644
>> --- a/cutils.c
>> +++ b/cutils.c
>> @@ -294,7 +294,8 @@ int fcntl_setfl(int fd, int flag)
>>  int64_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
>>  {
>>      int64_t retval = -1;
>> -    char *endptr, c, d;
>> +    char *endptr;
>> +    unsigned char c, d;
>>      int mul_required = 0;
>>      double val, mul, integral, fraction;
>>  
> 
> I doubt this hunk is still needed.

It isn't strictly due to qemu_toupper() but it is prettier to match the
real behavior of pure toupper() anyway.

Jes
Markus Armbruster - Jan. 24, 2011, 4:40 p.m.
Jes Sorensen <Jes.Sorensen@redhat.com> writes:

> On 01/24/11 17:10, Markus Armbruster wrote:
>> Jes.Sorensen@redhat.com writes:
>> 
>>> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>>>
>>> isspace() behavior is undefined for signed char.
>>>
>>> Bug pointed out by Eric Blake, thanks!
>>>
>>> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
>>> ---
>>>  cutils.c |    7 ++++---
>>>  1 files changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/cutils.c b/cutils.c
>>> index 4d2e27c..a067cf4 100644
>>> --- a/cutils.c
>>> +++ b/cutils.c
>>> @@ -294,7 +294,8 @@ int fcntl_setfl(int fd, int flag)
>>>  int64_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
>>>  {
>>>      int64_t retval = -1;
>>> -    char *endptr, c, d;
>>> +    char *endptr;
>>> +    unsigned char c, d;
>>>      int mul_required = 0;
>>>      double val, mul, integral, fraction;
>>>  
>> 
>> I doubt this hunk is still needed.
>
> It isn't strictly due to qemu_toupper() but it is prettier to match the
> real behavior of pure toupper() anyway.

Frequent casting between signed and unsigned has been shown to cause
headaches in laboratory rats.

Patch

diff --git a/cutils.c b/cutils.c
index 4d2e27c..a067cf4 100644
--- a/cutils.c
+++ b/cutils.c
@@ -294,7 +294,8 @@  int fcntl_setfl(int fd, int flag)
 int64_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
 {
     int64_t retval = -1;
-    char *endptr, c, d;
+    char *endptr;
+    unsigned char c, d;
     int mul_required = 0;
     double val, mul, integral, fraction;
 
@@ -314,7 +315,7 @@  int64_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
      */
     c = *endptr;
     d = c;
-    if (isspace(c) || c == '\0' || c == ',') {
+    if (qemu_isspace(c) || c == '\0' || c == ',') {
         c = 0;
         if (default_suffix) {
             d = default_suffix;
@@ -361,7 +362,7 @@  int64_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
      */
     if (c != 0) {
         endptr++;
-        if (!isspace(*endptr) && *endptr != ',' && *endptr != 0) {
+        if (!qemu_isspace(*endptr) && *endptr != ',' && *endptr != 0) {
             goto fail;
         }
     }