[Qemu-devel] [PATCH] Make strtosz() return int64_t instead of ssize_t

[Qemu-devel] [PATCH] Make strtosz() return int64_t instead of ssize_t

[Jes.Sorensen]

From: Jes Sorensen <Jes.Sorensen@redhat.com>

strtosz() needs to return a 64 bit type even on 32 bit
architectures. Otherwise qemu-img will fail to create disk
images >= 2GB

Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
---
 cutils.c      |    8 ++++----
 monitor.c     |    2 +-
 qemu-common.h |    4 ++--
 qemu-img.c    |    2 +-
 vl.c          |    4 ++--
 5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/cutils.c b/cutils.c
index 7984bc1..4d2e27c 100644
--- a/cutils.c
+++ b/cutils.c
@@ -291,9 +291,9 @@ int fcntl_setfl(int fd, int flag)
  * value must be terminated by whitespace, ',' or '\0'. Return -1 on
  * error.
  */
-ssize_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
+int64_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
 {
-    ssize_t retval = -1;
+    int64_t retval = -1;
     char *endptr, c, d;
     int mul_required = 0;
     double val, mul, integral, fraction;
@@ -365,7 +365,7 @@ ssize_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
             goto fail;
         }
     }
-    if ((val * mul >= ~(size_t)0) || val < 0) {
+    if ((val * mul >= INT64_MAX) || val < 0) {
         goto fail;
     }
     retval = val * mul;
@@ -378,7 +378,7 @@ fail:
     return retval;
 }
 
-ssize_t strtosz(const char *nptr, char **end)
+int64_t strtosz(const char *nptr, char **end)
 {
     return strtosz_suffix(nptr, end, STRTOSZ_DEFSUFFIX_MB);
 }
diff --git a/monitor.c b/monitor.c
index f258000..fcdae15 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4162,7 +4162,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
             break;
         case 'o':
             {
-                ssize_t val;
+                int64_t val;
                 char *end;
 
                 while (qemu_isspace(*p)) {
diff --git a/qemu-common.h b/qemu-common.h
index 63d9943..cce6e61 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -158,8 +158,8 @@ int fcntl_setfl(int fd, int flag);
 #define STRTOSZ_DEFSUFFIX_MB	'M'
 #define STRTOSZ_DEFSUFFIX_KB	'K'
 #define STRTOSZ_DEFSUFFIX_B	'B'
-ssize_t strtosz(const char *nptr, char **end);
-ssize_t strtosz_suffix(const char *nptr, char **end, const char default_suffix);
+int64_t strtosz(const char *nptr, char **end);
+int64_t strtosz_suffix(const char *nptr, char **end, const char default_suffix);
 
 /* path.c */
 void init_paths(const char *prefix);
diff --git a/qemu-img.c b/qemu-img.c
index afd9ed2..6af2a4c 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -320,7 +320,7 @@ static int img_create(int argc, char **argv)
 
     /* Get image size, if specified */
     if (optind < argc) {
-        ssize_t sval;
+        int64_t sval;
         sval = strtosz_suffix(argv[optind++], NULL, STRTOSZ_DEFSUFFIX_B);
         if (sval < 0) {
             error_report("Invalid image size specified! You may use k, M, G or "
diff --git a/vl.c b/vl.c
index 78fcef1..93425f4 100644
--- a/vl.c
+++ b/vl.c
@@ -804,7 +804,7 @@ static void numa_add(const char *optarg)
         if (get_param_value(option, 128, "mem", optarg) == 0) {
             node_mem[nodenr] = 0;
         } else {
-            ssize_t sval;
+            int64_t sval;
             sval = strtosz(option, NULL);
             if (sval < 0) {
                 fprintf(stderr, "qemu: invalid numa mem size: %s\n", optarg);
@@ -2245,7 +2245,7 @@ int main(int argc, char **argv, char **envp)
                 exit(0);
                 break;
             case QEMU_OPTION_m: {
-                ssize_t value;
+                int64_t value;
 
                 value = strtosz(optarg, NULL);
                 if (value < 0) {
-- 
1.7.3.4

[Qemu-devel] Re: [PATCH] Make strtosz() return int64_t instead of ssize_t

[Michael S. Tsirkin]

On Wed, Jan 05, 2011 at 11:41:02AM +0100, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> 
> strtosz() needs to return a 64 bit type even on 32 bit
> architectures. Otherwise qemu-img will fail to create disk
> images >= 2GB
> 
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>

Nothing wrong with this patch, but should the function
be renamed to strtos64 then?

> ---
>  cutils.c      |    8 ++++----
>  monitor.c     |    2 +-
>  qemu-common.h |    4 ++--
>  qemu-img.c    |    2 +-
>  vl.c          |    4 ++--
>  5 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/cutils.c b/cutils.c
> index 7984bc1..4d2e27c 100644
> --- a/cutils.c
> +++ b/cutils.c
> @@ -291,9 +291,9 @@ int fcntl_setfl(int fd, int flag)
>   * value must be terminated by whitespace, ',' or '\0'. Return -1 on
>   * error.
>   */
> -ssize_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
> +int64_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
>  {
> -    ssize_t retval = -1;
> +    int64_t retval = -1;
>      char *endptr, c, d;
>      int mul_required = 0;
>      double val, mul, integral, fraction;
> @@ -365,7 +365,7 @@ ssize_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
>              goto fail;
>          }
>      }
> -    if ((val * mul >= ~(size_t)0) || val < 0) {
> +    if ((val * mul >= INT64_MAX) || val < 0) {
>          goto fail;
>      }
>      retval = val * mul;
> @@ -378,7 +378,7 @@ fail:
>      return retval;
>  }
>  
> -ssize_t strtosz(const char *nptr, char **end)
> +int64_t strtosz(const char *nptr, char **end)
>  {
>      return strtosz_suffix(nptr, end, STRTOSZ_DEFSUFFIX_MB);
>  }
> diff --git a/monitor.c b/monitor.c
> index f258000..fcdae15 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -4162,7 +4162,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>              break;
>          case 'o':
>              {
> -                ssize_t val;
> +                int64_t val;
>                  char *end;
>  
>                  while (qemu_isspace(*p)) {
> diff --git a/qemu-common.h b/qemu-common.h
> index 63d9943..cce6e61 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -158,8 +158,8 @@ int fcntl_setfl(int fd, int flag);
>  #define STRTOSZ_DEFSUFFIX_MB	'M'
>  #define STRTOSZ_DEFSUFFIX_KB	'K'
>  #define STRTOSZ_DEFSUFFIX_B	'B'
> -ssize_t strtosz(const char *nptr, char **end);
> -ssize_t strtosz_suffix(const char *nptr, char **end, const char default_suffix);
> +int64_t strtosz(const char *nptr, char **end);
> +int64_t strtosz_suffix(const char *nptr, char **end, const char default_suffix);
>  
>  /* path.c */
>  void init_paths(const char *prefix);
> diff --git a/qemu-img.c b/qemu-img.c
> index afd9ed2..6af2a4c 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -320,7 +320,7 @@ static int img_create(int argc, char **argv)
>  
>      /* Get image size, if specified */
>      if (optind < argc) {
> -        ssize_t sval;
> +        int64_t sval;
>          sval = strtosz_suffix(argv[optind++], NULL, STRTOSZ_DEFSUFFIX_B);
>          if (sval < 0) {
>              error_report("Invalid image size specified! You may use k, M, G or "
> diff --git a/vl.c b/vl.c
> index 78fcef1..93425f4 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -804,7 +804,7 @@ static void numa_add(const char *optarg)
>          if (get_param_value(option, 128, "mem", optarg) == 0) {
>              node_mem[nodenr] = 0;
>          } else {
> -            ssize_t sval;
> +            int64_t sval;
>              sval = strtosz(option, NULL);
>              if (sval < 0) {
>                  fprintf(stderr, "qemu: invalid numa mem size: %s\n", optarg);
> @@ -2245,7 +2245,7 @@ int main(int argc, char **argv, char **envp)
>                  exit(0);
>                  break;
>              case QEMU_OPTION_m: {
> -                ssize_t value;
> +                int64_t value;
>  
>                  value = strtosz(optarg, NULL);
>                  if (value < 0) {
> -- 
> 1.7.3.4
> 

[Qemu-devel] Re: [PATCH] Make strtosz() return int64_t instead of ssize_t

[Jes Sorensen]

On 01/05/11 13:34, Michael S. Tsirkin wrote:
> On Wed, Jan 05, 2011 at 11:41:02AM +0100, Jes.Sorensen@redhat.com wrote:
>> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>>
>> strtosz() needs to return a 64 bit type even on 32 bit
>> architectures. Otherwise qemu-img will fail to create disk
>> images >= 2GB
>>
>> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> 
> Nothing wrong with this patch, but should the function
> be renamed to strtos64 then?

I don't think that adds any value to be honest. The problem with the old
interface was that the return type differed depending on whether it was
compiled on 32 vs 64 bit systems.

Cheers,
Jes

[Qemu-devel] Re: [PATCH] Make strtosz() return int64_t instead of ssize_t

[Michael S. Tsirkin]

On Wed, Jan 05, 2011 at 01:36:35PM +0100, Jes Sorensen wrote:
> On 01/05/11 13:34, Michael S. Tsirkin wrote:
> > On Wed, Jan 05, 2011 at 11:41:02AM +0100, Jes.Sorensen@redhat.com wrote:
> >> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> >>
> >> strtosz() needs to return a 64 bit type even on 32 bit
> >> architectures. Otherwise qemu-img will fail to create disk
> >> images >= 2GB
> >>
> >> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
> > 
> > Nothing wrong with this patch, but should the function
> > be renamed to strtos64 then?
> 
> I don't think that adds any value to be honest. The problem with the old
> interface was that the return type differed depending on whether it was
> compiled on 32 vs 64 bit systems.
> 
> Cheers,
> Jes

Yes, but does not the name strtosz imply 'string to size'?
Further changing the name will help make sure there are no
users you forgot to update.

-- 
MST

[Qemu-devel] Re: [PATCH] Make strtosz() return int64_t instead of ssize_t

[Anthony Liguori]

On 01/05/2011 04:41 AM, Jes.Sorensen@redhat.com wrote:
> From: Jes Sorensen<Jes.Sorensen@redhat.com>
>
> strtosz() needs to return a 64 bit type even on 32 bit
> architectures. Otherwise qemu-img will fail to create disk
> images>= 2GB
>
> Signed-off-by: Jes Sorensen<Jes.Sorensen@redhat.com>
>    

off_t would be the proper type to use.

Regards,

Anthony Liguori

> ---
>   cutils.c      |    8 ++++----
>   monitor.c     |    2 +-
>   qemu-common.h |    4 ++--
>   qemu-img.c    |    2 +-
>   vl.c          |    4 ++--
>   5 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/cutils.c b/cutils.c
> index 7984bc1..4d2e27c 100644
> --- a/cutils.c
> +++ b/cutils.c
> @@ -291,9 +291,9 @@ int fcntl_setfl(int fd, int flag)
>    * value must be terminated by whitespace, ',' or '\0'. Return -1 on
>    * error.
>    */
> -ssize_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
> +int64_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
>   {
> -    ssize_t retval = -1;
> +    int64_t retval = -1;
>       char *endptr, c, d;
>       int mul_required = 0;
>       double val, mul, integral, fraction;
> @@ -365,7 +365,7 @@ ssize_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
>               goto fail;
>           }
>       }
> -    if ((val * mul>= ~(size_t)0) || val<  0) {
> +    if ((val * mul>= INT64_MAX) || val<  0) {
>           goto fail;
>       }
>       retval = val * mul;
> @@ -378,7 +378,7 @@ fail:
>       return retval;
>   }
>
> -ssize_t strtosz(const char *nptr, char **end)
> +int64_t strtosz(const char *nptr, char **end)
>   {
>       return strtosz_suffix(nptr, end, STRTOSZ_DEFSUFFIX_MB);
>   }
> diff --git a/monitor.c b/monitor.c
> index f258000..fcdae15 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -4162,7 +4162,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>               break;
>           case 'o':
>               {
> -                ssize_t val;
> +                int64_t val;
>                   char *end;
>
>                   while (qemu_isspace(*p)) {
> diff --git a/qemu-common.h b/qemu-common.h
> index 63d9943..cce6e61 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -158,8 +158,8 @@ int fcntl_setfl(int fd, int flag);
>   #define STRTOSZ_DEFSUFFIX_MB	'M'
>   #define STRTOSZ_DEFSUFFIX_KB	'K'
>   #define STRTOSZ_DEFSUFFIX_B	'B'
> -ssize_t strtosz(const char *nptr, char **end);
> -ssize_t strtosz_suffix(const char *nptr, char **end, const char default_suffix);
> +int64_t strtosz(const char *nptr, char **end);
> +int64_t strtosz_suffix(const char *nptr, char **end, const char default_suffix);
>
>   /* path.c */
>   void init_paths(const char *prefix);
> diff --git a/qemu-img.c b/qemu-img.c
> index afd9ed2..6af2a4c 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -320,7 +320,7 @@ static int img_create(int argc, char **argv)
>
>       /* Get image size, if specified */
>       if (optind<  argc) {
> -        ssize_t sval;
> +        int64_t sval;
>           sval = strtosz_suffix(argv[optind++], NULL, STRTOSZ_DEFSUFFIX_B);
>           if (sval<  0) {
>               error_report("Invalid image size specified! You may use k, M, G or "
> diff --git a/vl.c b/vl.c
> index 78fcef1..93425f4 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -804,7 +804,7 @@ static void numa_add(const char *optarg)
>           if (get_param_value(option, 128, "mem", optarg) == 0) {
>               node_mem[nodenr] = 0;
>           } else {
> -            ssize_t sval;
> +            int64_t sval;
>               sval = strtosz(option, NULL);
>               if (sval<  0) {
>                   fprintf(stderr, "qemu: invalid numa mem size: %s\n", optarg);
> @@ -2245,7 +2245,7 @@ int main(int argc, char **argv, char **envp)
>                   exit(0);
>                   break;
>               case QEMU_OPTION_m: {
> -                ssize_t value;
> +                int64_t value;
>
>                   value = strtosz(optarg, NULL);
>                   if (value<  0) {
>    

[Qemu-devel] Re: [PATCH] Make strtosz() return int64_t instead of ssize_t

[Jes Sorensen]

On 01/05/11 14:46, Anthony Liguori wrote:
> On 01/05/2011 04:41 AM, Jes.Sorensen@redhat.com wrote:
>> From: Jes Sorensen<Jes.Sorensen@redhat.com>
>>
>> strtosz() needs to return a 64 bit type even on 32 bit
>> architectures. Otherwise qemu-img will fail to create disk
>> images>= 2GB
>>
>> Signed-off-by: Jes Sorensen<Jes.Sorensen@redhat.com>
>>    
> 
> off_t would be the proper type to use.

I discussed this with Markus, and we both agree that it isn't.

Two reasons, off_t is for filesystem offsets, not random sizes. Second,
off_t doesn't have an unsigned type or a max to compare against.

Using int64_t is cleaner and safer.

Cheers,
Jes

[Qemu-devel] Re: [PATCH] Make strtosz() return int64_t instead of ssize_t

[Anthony Liguori]

On 01/05/2011 08:40 AM, Jes Sorensen wrote:
> On 01/05/11 14:46, Anthony Liguori wrote:
>    
>> On 01/05/2011 04:41 AM, Jes.Sorensen@redhat.com wrote:
>>      
>>> From: Jes Sorensen<Jes.Sorensen@redhat.com>
>>>
>>> strtosz() needs to return a 64 bit type even on 32 bit
>>> architectures. Otherwise qemu-img will fail to create disk
>>> images>= 2GB
>>>
>>> Signed-off-by: Jes Sorensen<Jes.Sorensen@redhat.com>
>>>
>>>        
>> off_t would be the proper type to use.
>>      
> I discussed this with Markus, and we both agree that it isn't.
>
> Two reasons, off_t is for filesystem offsets, not random sizes. Second,
> off_t doesn't have an unsigned type or a max to compare against.
>    

That's because the size of off_t depends on whether it's a 32 or 64-bit 
platform and what FILESIZEBITS is defined to be.

Basically, if you're looking for the type to represent offsets in a 
file, it's off_t.  That's why it exists.

That said, using this to represent memory too, I can buy that as a 
justification to use int64_t.

> Using int64_t is cleaner and safer.
>    

I wouldn't make such bold claims but I'll concede that one is not 
significantly better than the other and won't object to int64_t if you 
feel strongly.

Regards,

Anthony Liguori

> Cheers,
> Jes
>    

[Qemu-devel] Re: [PATCH] Make strtosz() return int64_t instead of ssize_t

[Jes Sorensen]

On 01/05/11 19:29, Anthony Liguori wrote:
> I wouldn't make such bold claims but I'll concede that one is not
> significantly better than the other and won't object to int64_t if you
> feel strongly.

The more I think of it, the more I come to the conclusion that int64_t
is the best solution. Since we can in theory have a system where we only
allow 32 bit file system offsets, but have > 4GB of memory, int64_t does
it best - it is more generic.

Cheers,
Jes

Re: [Qemu-devel] [PATCH] Make strtosz() return int64_t instead of ssize_t

[Kevin Wolf]

Am 05.01.2011 11:41, schrieb Jes.Sorensen@redhat.com:
> From: Jes Sorensen <Jes.Sorensen@redhat.com>
> 
> strtosz() needs to return a 64 bit type even on 32 bit
> architectures. Otherwise qemu-img will fail to create disk
> images >= 2GB
> 
> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>

Thanks, applied to the block branch.

Kevin