Hi Ali, From: Ali Farzanrad <[email protected]> Date: Thu, 24 Jun 2021 07:25:24 +0000
> Hi Masato, > > Masato Asou <[email protected]> wrote: >> Hi Ali, >> >> In this case, ULLONG_MAX is implicitly cast to double, isn't it? > > Yes it is implicitly cast to double, but due to floating point number > precision, ULLONG_MAX will convert to (ULLONG_MAX + 1). > So in case val is really ULLONG_MAX + 1, this condition is false: > > if (val > ULLONG_MAX) > > and when we want to convert val to u_int64_t we will have > (ULLONG_MAX + 1) which is 0. > > *n = val; /* stores ULLONG_MAX + 1 which is 0 */ I was not aware of that problem. Thank you for the explanation. > I'm not sure if it cause any real problem, but it is wrong and should be > fixed. It is true that I don't know what specific actual problem will occur, but there is a potential problem. -- ASOU Masato >> >> Do you have any problems if you don't cast to double? >> -- >> ASOU Masato >> >> From: Ali Farzanrad <[email protected]> >> Date: Wed, 23 Jun 2021 20:24:27 +0000 >> >> > Oh, my bad, sorry. >> > I assumed that val is always integer. >> > I guess it is better to ignore val == ULLONG_MAX: >> > >> > =================================================================== >> > RCS file: /cvs/src/sbin/disklabel/editor.c,v >> > retrieving revision 1.368 >> > diff -u -p -r1.368 editor.c >> > --- editor.c 2021/05/30 19:02:30 1.368 >> > +++ editor.c 2021/06/23 20:23:03 >> > @@ -2418,7 +2418,7 @@ apply_unit(double val, u_char unit, u_int64_t *n) >> > } >> > >> > val *= factor / DEV_BSIZE; >> > - if (val > ULLONG_MAX) >> > + if (val >= (double)ULLONG_MAX) >> > return (-1); >> > *n = val; >> > return (0); >> > @@ -2429,7 +2429,7 @@ parse_sizespec(const char *buf, double *val, char >> > **un >> > { >> > errno = 0; >> > *val = strtod(buf, unit); >> > - if (errno == ERANGE || *val < 0 || *val > ULLONG_MAX) >> > + if (errno == ERANGE || *val < 0 || *val >= (double)ULLONG_MAX) >> > return (-1); /* too big/small */ >> > if (*val == 0 && *unit == buf) >> > return (-1); /* No conversion performed. */ >> > >> > Ali Farzanrad <[email protected]> wrote: >> >> Hi tech@, >> >> >> >> disklabel shows a warning at build time which might be important. >> >> Following diff will surpass the warning. >> >> >> >> =================================================================== >> >> RCS file: /cvs/src/sbin/disklabel/editor.c,v >> >> retrieving revision 1.368 >> >> diff -u -p -r1.368 editor.c >> >> --- editor.c 2021/05/30 19:02:30 1.368 >> >> +++ editor.c 2021/06/23 19:25:45 >> >> @@ -2418,7 +2418,7 @@ apply_unit(double val, u_char unit, u_int64_t *n) >> >> } >> >> >> >> val *= factor / DEV_BSIZE; >> >> - if (val > ULLONG_MAX) >> >> + if (val != (double)(u_int64_t)val) >> >> return (-1); >> >> *n = val; >> >> return (0); >> >> @@ -2429,7 +2429,7 @@ parse_sizespec(const char *buf, double *val, char >> >> **un >> >> { >> >> errno = 0; >> >> *val = strtod(buf, unit); >> >> - if (errno == ERANGE || *val < 0 || *val > ULLONG_MAX) >> >> + if (errno == ERANGE || *val < 0 || *val != (double)(u_int64_t)*val) >> >> return (-1); /* too big/small */ >> >> if (*val == 0 && *unit == buf) >> >> return (-1); /* No conversion performed. */ >> >> >> >> >> > >> >>
