Hello, On Sat, Jun 22, 2024 at 03:13:22PM +0900, yskelg@xxxxxxxxx wrote: > From: Yunseong Kim <yskelg@xxxxxxxxx> > > This patch fix compiler warning in kmod_dump_index() > > libkmod/libkmod.c:989:11: warning: result of comparison of unsigned enum > expression < 0 is always false [-Wtautological-unsigned-enum-zero-compare] > 989 | if (type < 0 || type >= _KMOD_INDEX_MODULES_SIZE) > | ~~~~ ^ ~ > > My compiler version. > > $ clang -v > clang version 18.1.6 (Fedora 18.1.6-3.fc40) If you look eg. here https://stackoverflow.com/questions/2579230/signedness-of-enum-in-c-c99-c-cx-gnu-c-gnu-c99 you can see that it is not *guaranteed* that the enum is unsigend, and consequently a value lower than zero can be passed in. Consequently the bug is in your compiler issuing the warning. While in that specific C implementation the value cannot be negative in general it can. Finally I see that _KMOD_INDEX_MODULES_SIZE is defined separately from the enumeration itself, and would be prone to getting slale if the enumeration is ever updated. Making it part of the enumeration so it automatically gets the value of last used index type incremented by one would be probably less error-prone. In the kernel there is also a pattern of defining *_LAST as an alias to the last used value in the enumeration, and then _KMOD_INDEX_MODULES_SIZE coud be defined as this last value incremented by 1. Thanks Michal > > Signed-off-by: Yunseong Kim <yskelg@xxxxxxxxx> > --- > libkmod/libkmod.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libkmod/libkmod.c b/libkmod/libkmod.c > index 213b424..fcb619b 100644 > --- a/libkmod/libkmod.c > +++ b/libkmod/libkmod.c > @@ -986,7 +986,7 @@ KMOD_EXPORT int kmod_dump_index(struct kmod_ctx *ctx, enum kmod_index type, > if (ctx == NULL) > return -ENOSYS; > > - if (type < 0 || type >= _KMOD_INDEX_MODULES_SIZE) > + if (type >= KMOD_INDEX_MODULES_DEP || type >= _KMOD_INDEX_MODULES_SIZE) Why are you adding a duplicate check here? > return -ENOENT; > > if (ctx->indexes[type] != NULL) { > -- > 2.45.2 >
