Hello Bart, On Sat, 2019-10-12 at 15:59 -0700, Bart Van Assche wrote: > On 2019-10-12 14:28, Martin Wilck wrote: > > - if (snprintf(buf, bufsiz, "%s%s%d", mapname, delim, part) >= > > bufsiz) > > + if (snprintf(buf, bufsiz, "%s%s%d", mapname, delim, part) > > + >= (int)bufsiz) > > return 0; > > Please don't insert casts like this. I think enabling -Wsign-compare > is > wrong because it makes the source code ugly. The casts make it explicit which signedness is intended, which is a good thing IMO, better than the compiler trying to figure it out using implicit type conversion. Enabling the warning will help avoid subtle programming errors in the future, by forcing us to think twice about signedness. Considering that, isn't this ugliness - which I don't dispute - a relatively minor issue? In this particular case, we're dealing with a historically-caused property of snprintf(), as you are probably aware (https://stackoverflow.com/questions/45740276/why-does-printf-return-an-int-instead-of-a-size-t-in-c), so I'd argue the ugliness is forced upon us, sort of. We can hide the ugliness in a macro if you prefer. Actually, we have safe_snprintf() already. We just need to use it in all places where this kind of comparison is made. Would that be acceptable to you? Thanks, Martin -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
