On Sun, Nov 15, 2015 at 09:38:34AM -0500, Ganesh Ajjanagadde wrote:
> 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.i belive checking user input for validity is important so the user knows if there is a problem and where it is instead of just something not working and its very hard to predict which check is not usefull there are a wide array of possible typos, from missing wrong or superflous seperators to all kinds of wrong chars, copy and paste errors, mistakenly duplicated values, ... [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Rewriting code that is poorly written but fully understood is good. Rewriting code that one doesnt understand is a sign that one is less smart then the original author, trying to rewrite it will not make it better.
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
