| Message ID | 20241127162904.28182-1-advaitdhamorikar@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [PATCH-next] modpost: Remove logically dead condition | expand |
On Wed, Nov 27, 2024 at 09:59:04PM +0530 Advait Dhamorikar wrote: > In case of failure vsnprintf returns `pos`, an unsigned long integer. > An unsigned value can never be negative, so this test will always evaluate > the same way. 'man vsnprintf' on my system reveals a different behaviour: | The functions snprintf() and vsnprintf() do not | write more than size bytes (including the termi‐ | nating null byte ('\0')). If the output was | truncated due to this limit, then the return | value is the number of characters (excluding the | terminating null byte) which would have been | written to the final string if enough space had | been available. Thus, a return value of size or | more means that the output was truncated. (See | also below under NOTES.) | | If an output error is encountered, a negative | value is returned. vsnprintf prototypes also indicate 'int' as return type. What is the source of your mentioned findings? Kind regards, Nicolas
Hello Nicolas, > vsnprintf prototypes also indicate 'int' as return type. What is the source of your mentioned findings? Sorry, I read an alternate vsnprintf implementation and have worded my patch log wrong based on it. However there is still an issue that n is declared as size_t which is a typedef for an unsigned long, I think the correct solution then is to use a signed data type here for n? Thanks for your time and feedback. Best regards, Advait On Thu, 28 Nov 2024 at 02:21, Nicolas Schier <nicolas@fjasle.eu> wrote: > > On Wed, Nov 27, 2024 at 09:59:04PM +0530 Advait Dhamorikar wrote: > > In case of failure vsnprintf returns `pos`, an unsigned long integer. > > An unsigned value can never be negative, so this test will always evaluate > > the same way. > > 'man vsnprintf' on my system reveals a different behaviour: > > | The functions snprintf() and vsnprintf() do not > | write more than size bytes (including the termi‐ > | nating null byte ('\0')). If the output was > | truncated due to this limit, then the return > | value is the number of characters (excluding the > | terminating null byte) which would have been > | written to the final string if enough space had > | been available. Thus, a return value of size or > | more means that the output was truncated. (See > | also below under NOTES.) > | > | If an output error is encountered, a negative > | value is returned. > > vsnprintf prototypes also indicate 'int' as return type. What is the source of your mentioned findings? > > Kind regards, > Nicolas >
On Thu, Nov 28, 2024 at 6:31 AM Advait Dhamorikar <advaitdhamorikar@gmail.com> wrote: > > Hello Nicolas, > > > vsnprintf prototypes also indicate 'int' as return type. What is the source of your mentioned findings? > Sorry, I read an alternate vsnprintf implementation and have worded my > patch log wrong based on it. > > However there is still an issue that n is declared as size_t which is > a typedef for > an unsigned long, I think the correct solution then is to use a signed > data type here for n? Yes. 'n' should be int. This matches the return type of vsnprintf(). I will squash the following. diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c index 81f20ef13a0d..5b5745f00eb3 100644 --- a/scripts/mod/file2alias.c +++ b/scripts/mod/file2alias.c @@ -49,7 +49,8 @@ module_alias_printf(struct module *mod, bool append_wildcard, const char *fmt, ...) { struct module_alias *new, *als; - size_t len, n; + size_t len; + int n; va_list ap; /* Determine required size. */ Thank you for your report! > > Thanks for your time and feedback. > > Best regards, > Advait > > On Thu, 28 Nov 2024 at 02:21, Nicolas Schier <nicolas@fjasle.eu> wrote: > > > > On Wed, Nov 27, 2024 at 09:59:04PM +0530 Advait Dhamorikar wrote: > > > In case of failure vsnprintf returns `pos`, an unsigned long integer. > > > An unsigned value can never be negative, so this test will always evaluate > > > the same way. > > > > 'man vsnprintf' on my system reveals a different behaviour: > > > > | The functions snprintf() and vsnprintf() do not > > | write more than size bytes (including the termi‐ > > | nating null byte ('\0')). If the output was > > | truncated due to this limit, then the return > > | value is the number of characters (excluding the > > | terminating null byte) which would have been > > | written to the final string if enough space had > > | been available. Thus, a return value of size or > > | more means that the output was truncated. (See > > | also below under NOTES.) > > | > > | If an output error is encountered, a negative > > | value is returned. > > > > vsnprintf prototypes also indicate 'int' as return type. What is the source of your mentioned findings? > > > > Kind regards, > > Nicolas > >
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c index 81f20ef13a0d..8ce48d7dd36d 100644 --- a/scripts/mod/file2alias.c +++ b/scripts/mod/file2alias.c @@ -57,11 +57,6 @@ module_alias_printf(struct module *mod, bool append_wildcard, n = vsnprintf(NULL, 0, fmt, ap); va_end(ap); - if (n < 0) { - error("vsnprintf failed\n"); - return; - } - len = n + 1; /* extra byte for '\0' */ if (append_wildcard)
In case of failure vsnprintf returns `pos`, an unsigned long integer. An unsigned value can never be negative, so this test will always evaluate the same way. Signed-off-by: Advait Dhamorikar <advaitdhamorikar@gmail.com> --- scripts/mod/file2alias.c | 5 ----- 1 file changed, 5 deletions(-)