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

[Stefano Sabatini]

On date Sunday 2008-02-17 11:27:23 -0500, D. Hugh Redelmeier encoded:
> | From: Stefano Sabatini <stefano.sabatini-lala at poste.it>
> 
> | as in $subject, relevant thread:
> | http://thread.gmane.org/alpine.LRH.1.00.0802150032240.23194 at redclaw.mimosa.com
> 
> There is one bug that I noticed in reading your patch:  errno MIGHT be
> set to ERANGE due to a previous library call.  Before calling strtol,
> the code should initialize errno to zero.
> 
> This is explained in the NOTES section of the strtol manpage on my
> system.

Good catch, thanks.
 
> Your first patch also replaced a bunch of atoi calls with calls to
> parse_int_or_die.  I hope you submit a patch to do that too.

Yes, if this get applied I'll send further patches to apply
parse_int_or_die to the various ff* tools (where is actually used atoi
instead).

> I still like the idea of a context parameter to parse_int_or_die (or
> to another similar function).  At least most of the atoi call sites in
> parse_int_or_die did seem to know enough to specify a context (for
> example, "left crop size").
> 
> At least eight of these call sites check for and reject negative
> numbers.  This suggests that a parse_nat_or_die would pay for itself
> by factoring out those checks for negative numbers.  Without a context
> argument, the diagnostic would be worse than the current one.

I finally convinced myself of this solution, only now other functions
should also be changed to make them consisten with this (for example
parse_time_or_die).

> Thanks!

Thanks for your review, regards.
-- 
Stefano Sabatini
Linux user number 337176 (see http://counter.li.org)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: implement-parse-int-or-die-01.patch
Type: text/x-diff
Size: 1386 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080217/06b9e22b/attachment.patch>