On Sat, Nov 14, 2015 at 4:12 PM, Michael Niedermayer <mich...@niedermayer.cc> wrote: > 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 <gajjanaga...@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.
The first one does not need to be, it is deprecated syntax and anyway the only value it cares about is 0 versus nonzero. The second one can be range checked. Honestly though, I do not favor adding cruft like this for minimal gain since for all practical usages, no one will pass a value >= 2^64 in absolute value. For me, what is important is avoiding the undefined behavior. If you or others still feel that they need to be range checked, will do so. > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Everything should be made as simple as possible, but not simpler. > -- Albert Einstein > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel