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

Re: [libvirt] [PATCH 08/26] virfile: Resolve Coverity DEADCODE



Well, the bug here is a bit more serious than mere DEADCODE ...

On 09/05/14 00:26, John Ferlan wrote:
> Adjust the parentheses in/for the waitpid loops; otherwise, Coverity
> points out:
> 
> (1) Event assignment:   Assigning: "waitret" = "waitpid(pid, &status, 0) == -1"
> (2) Event between:      At condition "waitret == -1", the value of "waitret"
>                         must be between 0 and 1.
> (3) Event dead_error_condition:     The condition "waitret == -1" cannot
>                         be true.
> (4) Event dead_error_begin:     Execution cannot reach this statement:
>                         "ret = -*__errno_location();".
> 
> Signed-off-by: John Ferlan <jferlan redhat com>
> ---
>  src/util/virfile.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index cfb6cc1..b602144 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -2072,8 +2072,7 @@ virFileOpenForked(const char *path, int openflags, mode_t mode,
>      }
>  
>      /* wait for child to complete, and retrieve its exit code */
> -    while ((waitret = waitpid(pid, &status, 0) == -1)
> -           && (errno == EINTR));

This existing code worked by chance as the return value of waitpid was
compared to -1 and then assigned to waitret. ..also that part was used
in the comparison. If the return value was -1 then it would correctly loop.

> +    while ((waitret = waitpid(pid, &status, 0)) == -1 && (errno == EINTR));

the errno == EINTR doesn't need parentheses

>      if (waitret == -1) {

But this condition couldn't be reached.

>          ret = -errno;
>          virReportSystemError(errno,
> @@ -2290,7 +2289,7 @@ virDirCreate(const char *path,
>      if (pid) { /* parent */
>          /* wait for child to complete, and retrieve its exit code */
>          VIR_FREE(groups);
> -        while ((waitret = waitpid(pid, &status, 0) == -1)  && (errno == EINTR));
> +        while ((waitret = waitpid(pid, &status, 0)) == -1 && (errno == EINTR));

Same issue here.

>          if (waitret == -1) {
>              ret = -errno;
>              virReportSystemError(errno,
> 

ACK

Attachment: signature.asc
Description: OpenPGP digital signature


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