[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 6/9] virtime: Resolve Coverity DEADCODE



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 redhat com>
> ---

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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]