[FFmpeg-devel] [PATCH 2/2] ffserver_config: replace strtod by av_strtod and correct undefined behavior

[Michael Niedermayer]

On Thu, Nov 12, 2015 at 09:46:05PM -0500, Ganesh Ajjanagadde wrote:
> This uses av_strtod for added flexibility, and av_rint64_clip for ensuring that
> no undefined behavior gets invoked.
> 
> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
> ---
>  ffserver_config.c | 21 +++++----------------
>  1 file changed, 5 insertions(+), 16 deletions(-)
> 
> diff --git a/ffserver_config.c b/ffserver_config.c
> index 9fc1f00..443e71d 100644
> --- a/ffserver_config.c
> +++ b/ffserver_config.c
> @@ -19,6 +19,7 @@
>   */
>  
>  #include <float.h>
> +#include "libavutil/eval.h"
>  #include "libavutil/opt.h"
>  #include "libavutil/parseutils.h"
>  #include "libavutil/avstring.h"
> @@ -757,7 +758,7 @@ static int ffserver_parse_config_feed(FFServerConfig *config, const char *cmd,
>          } else {
>              WARNING("Truncate N syntax in configuration file is deprecated. "
>                      "Use Truncate alone with no arguments.\n");
> -            feed->truncate = strtod(arg, NULL);
> +            feed->truncate = av_rint64_clip(av_strtod(arg, NULL), INT_MIN, INT_MAX);
>          }
>      } else if (!av_strcasecmp(cmd, "FileMaxSize")) {
>          char *p1;
> @@ -765,22 +766,10 @@ static int ffserver_parse_config_feed(FFServerConfig *config, const char *cmd,
>  
>          ffserver_get_arg(arg, sizeof(arg), p);
>          p1 = arg;
> -        fsize = strtod(p1, &p1);
> -        switch(av_toupper(*p1)) {
> -        case 'K':
> -            fsize *= 1024;
> -            break;
> -        case 'M':
> -            fsize *= 1024 * 1024;
> -            break;
> -        case 'G':
> -            fsize *= 1024 * 1024 * 1024;
> -            break;
> -        default:
> +        fsize = av_strtod(p1, &p1);
> +        if (!fsize || fabs(fsize) == HUGE_VAL)
>              ERROR("Invalid file size: '%s'\n", arg);
> -            break;
> -        }
> -        feed->feed_max_size = (int64_t)fsize;
> +        feed->feed_max_size = av_rint64_clip(fsize, INT64_MIN, INT64_MAX);

i think these should be range checked and causing errors or warnings
if they are out of range

if the user asks for value X and we cant do that then we should tell
the user that we cant.

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

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151114/b5eaeb08/attachment.sig>