[FFmpeg-devel] [PATCH] Implement the function cmdutils.c:parse_int_or_die

[Michael Niedermayer]

On Sun, Feb 17, 2008 at 08:50:19PM +0100, Stefano Sabatini wrote:
> On date Sunday 2008-02-17 13:40:47 -0500, D. Hugh Redelmeier encoded:
> > | From: Michael Niedermayer <michaelni at gmx.at>
> > 
> > | Id prefer
> > | double parse_number_or_die(const char* numstr, int type, double min, double max, const char* error);
> > | 
> > | d= parse_number_or_die("12234", OPT_INT, 1, 100, "blah blah");
> > 
> > I really really distrust using floating point when integers are
> > expected.  (To be honest, I distrust using floating point for anything
> > where an exact result is expected.)
> > 
> > Do we really want to allow "3.9" when the context calls for an integer?
> > 
> > Theoretically, a C implementation could have some ints that are not
> > representable as doubles.  I don't think that such an implementation
> > exists, but it still makes me uncomforable.
> 
> Attached patch proposes a solution which implements MN suggestion, for
> what regards the double to int conversion we could simply split the
> function int three different ones, like:
> 
> int parse_int_or_die(...)
> int64_t parse_int64_or_die(...)
> float parse_float_or_die(...)
> 
> Regards.
> -- 
> Stefano Sabatini
> Linux user number 337176 (see http://counter.li.org)

> Index: cmdutils.h
> ===================================================================
> --- cmdutils.h	(revision 12129)
> +++ cmdutils.h	(working copy)
> @@ -89,4 +89,18 @@
>   */
>  void show_license(void);
>  
> +/**
> + * parses a string and returns its corresponding numeric value or
> + * exits from the application if the string cannot be correctly parsed
> + * or the corresponding value is invalid
> + *
> + * @param context the context of the value to be setted (e.g. the
> + * corresponding commandline option name)
> + * @param numstr the string to be parsed
> + * @param type the type of the variable where to set the resulting value
> + * @param min the minimum valid value accepted
> + * @param max the maximum valid value accepted
> + */
> +double parse_number_or_die(const char *context, const char* numstr, int type, double min, double max);
> +
>  #endif /* FFMPEG_CMDUTILS_H */
> Index: cmdutils.c
> ===================================================================
> --- cmdutils.c	(revision 12129)
> +++ cmdutils.c	(working copy)
> @@ -22,6 +22,7 @@
>  #include <string.h>
>  #include <stdlib.h>
>  #include <errno.h>
> +#include <math.h>
>  
>  #include "avformat.h"
>  #include "avdevice.h"
> @@ -64,6 +65,69 @@
>      return po;
>  }
>  
> +double parse_number_or_die(const char *context, const char* numstr, int type, double min, double max)
> +{
> +    double d;
> +    long int li;
> +    long long int lli;
> +    char *tail;
> +
> +    /* errno could be already setted by previous library calls, so reset it */
> +    errno = 0;
> +
> +    switch(type) {
> +    case OPT_INT:
> +        li = strtol(numstr, &tail, 10);
> +        if (*tail != '\0') {
> +            fprintf(stderr, "Expected integer for %s but found: %s\n", context, numstr);
> +            exit(1);
> +        } else if (errno == ERANGE || li < INT_MIN || li > INT_MAX) {
> +            fprintf(stderr, "Value for %s is too big for an int: %s\n", context, numstr);
> +            exit(1);
> +        } else if (li < min || li > max) {
> +            fprintf(stderr, "Value for %s has be <= to %ld and <= %ld: %s\n", context, (long int)min, (long int)max, numstr);
> +            exit(1);
> +        }
> +        return li;
> +        break;
> +
> +    case OPT_INT64:
> +        lli = strtoll(numstr, &tail, 10);
> +        if (*tail != '\0') {
> +            fprintf(stderr, "Expected integer for %s but found: %s\n", context, numstr);
> +            exit(1);
> +        } else if (errno == ERANGE || lli < INT64_MIN || lli > INT64_MAX) {
> +            fprintf(stderr, "Value for %s is too big for an int64_t: %s\n", context, numstr);
> +            exit(1);
> +        } else if (lli < min || lli > max) {
> +            fprintf(stderr, "Value for %s has be <= to %lld and <= %lld: %s\n", context, (long long int)min, (long long int)max, numstr);
> +            exit(1);
> +        }
> +        return lli;
> +        break;

theres no need for both int and int64
and the whole ERANGE thing is annoying, the max/min checks are enough
and int64 will not be <INT64_MIN nor >INT64_MAX

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080217/61a8b1aa/attachment.pgp>