On Wed, Dec 06, 2023 at 21:30:59 +0300, Anastasia Belova wrote:
> virFileLinkPointsTo return non-zero value on fail. This value
> may be positive or negative, so check should be != 0.
This statement is not true:
#define SAME_INODE(Stat_buf_1, Stat_buf_2) \
((Stat_buf_1).st_ino == (Stat_buf_2).st_ino \
&& (Stat_buf_1).st_dev == (Stat_buf_2).st_dev)
/* Return nonzero if checkLink and checkDest
* refer to the same file. Otherwise, return 0.
*/
int
virFileLinkPointsTo(const char *checkLink,
const char *checkDest)
{
struct stat src_sb;
struct stat dest_sb;
return (stat(checkLink, &src_sb) == 0
&& stat(checkDest, &dest_sb) == 0
&& SAME_INODE(src_sb, dest_sb));
}
Based on the docs and the code virFileLinkPointsTo returns 'true'
typecast to int (thus non-zero) on success, if the ling (1st arg) points
to the second arg. Otherwise if the state can't be determined or the
linking doesn't exist it returns false, thus 0.
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Fixes: bddbda99df ("network: Introduce virnetworkobj")
> Signed-off-by: Anastasia Belova <abelova@xxxxxxxxxxxxx>
> ---
> src/conf/virnetworkobj.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
> index 20ee8eb58a..47658986e8 100644
> --- a/src/conf/virnetworkobj.c
> +++ b/src/conf/virnetworkobj.c
> @@ -945,7 +945,7 @@ virNetworkLoadConfig(virNetworkObjList *nets,
> if ((autostartLink = virNetworkConfigFile(autostartDir, name)) == NULL)
> return NULL;
>
> - if ((autostart = virFileLinkPointsTo(autostartLink, configFile)) < 0)
> + if ((autostart = virFileLinkPointsTo(autostartLink, configFile)) != 0)
> return NULL;
Thus doing this would cause to always bail out as if it were failure if
the link points to the expected file.
In fact the original behaviour, which would be that the condition is
always false => dead code was correct, and the condition should be
removed.
'autostart' can be filled unconditionally.
In fact for a proper fix virFileLinkPointsTo should be also converted to
return bool.
>
> if (!(def = virNetworkDefParse(NULL, configFile, xmlopt, false)))
> --
> 2.30.2
> _______________________________________________
> Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
> To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
