On 09/12/14 02:06, John Ferlan wrote: > Coverity complains that because of how 'offset' is initialized to > 0 (zero), the resulting math and comparison on rem is pointless. > > For the "while (rem < 0)", the value of 'rem' must be between > 0 and 86399 (SECS_PER_DAY = 86400ULL). Thus, the addition of > offset (0) does nothing and the while (rem < 0) is pointless. > > For the "while (rem > SECS_PER_DAY)", we have the same issue. > > Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> > --- From your cover letter: 6 -> virTimeFieldsThen() and the "offset = 0". I'd be OK with deleting the code, but it just feels like someone had it on a todo list to come back to some day According to the comment in the code: void virTimeFieldsThen(unsigned long long when, struct tm *fields) { /* This code is taken from GLibC under terms of LGPLv2+ */ long int days, rem, y; it was borrowed from glibc. AFAIK we decided to always use GMT in our time stamps and everywhere so it'll be probably better just to delete the code and note that a part is missing. I doubt that anyone will be adding it later. > src/util/virtime.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/src/util/virtime.c b/src/util/virtime.c > index 9fefb67..99c7cf6 100644 > --- a/src/util/virtime.c > +++ b/src/util/virtime.c > @@ -135,10 +135,12 @@ void virTimeFieldsThen(unsigned long long when, struct tm *fields) > days = whenSecs / SECS_PER_DAY; > rem = whenSecs % SECS_PER_DAY; > rem += offset; > + /* coverity[dead_error_condition] - when offset is calculated remove this */ > while (rem < 0) { > rem += SECS_PER_DAY; > --days; > } > + /* coverity[dead_error_condition] - when offset is calculated remove this */ > while (rem >= SECS_PER_DAY) { > rem -= SECS_PER_DAY; > ++days; > Anyways, if somebody speaks against deleting the part, then ACK to this approach. Peter
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list