Re: [PATCH v2 2/6] openvz: Resolve Coverity FORWARD_NULL

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

 



On Fri, Sep 25, 2015 at 12:31:41 -0400, John Ferlan wrote:
> Coverity complains that strtok_r cannot be called if the first and third
> parameters are NULL. On entry if the 'value' parameter is not checked for
> being NULL before the VIR_STRDUP which will set the target to NULL if
> the source (eg value) is NULL.  Thus add a check for NULL value.
> 
> NB: Although each of the callers to openvzParseBarrierLimit would only
> make the call when 'value' was non-NULL, it seems that doesn't satisfy
> the checker. The NULL check had to be in this funtion.

Erm, so then file a bug against coverity rather than hacking around it?

> 
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
>  src/openvz/openvz_conf.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
> index fb8768a..e99b83c 100644
> --- a/src/openvz/openvz_conf.c
> +++ b/src/openvz/openvz_conf.c
> @@ -135,10 +135,10 @@ openvzParseBarrierLimit(const char* value,
>  {
>      char *token;
>      char *saveptr = NULL;
> -    char *str;
> +    char *str = NULL;
>      int ret = -1;
>  
> -    if (VIR_STRDUP(str, value) < 0)
> +    if (!value || VIR_STRDUP(str, value) < 0)

VIR_STRDUP returns 0 if @value was NULL so you can as well as test for <
1. Since then the value gets initialized to NULL you don't need the
first hunk.

>          goto error;
>  
>      token = strtok_r(str, ":", &saveptr);
> @@ -396,7 +396,7 @@ openvzReadFSConf(virDomainDefPtr def,
>      param = "DISKSPACE";
>      ret = openvzReadVPSConfigParam(veid, param, &temp);
>      if (ret > 0) {
> -        if (openvzParseBarrierLimit(temp, &barrier, &limit)) {
> +        if (openvzParseBarrierLimit(temp, &barrier, &limit) < 0) {
>              virReportError(VIR_ERR_INTERNAL_ERROR,
>                             _("Could not read '%s' from config for container %d"),
>                             param, veid);

This change should be mentioned in the commit message since it's
different from what the current message says.

Peter

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]