| Message ID | 1491565649-16679-1-git-send-email-James.Cowgill@imgtec.com |
|---|---|
| State | Accepted |
| Delegated to: | Pablo Neira |
| Headers | show |
On 7 April 2017 at 13:47, James Cowgill <James.Cowgill@imgtec.com> wrote: > hashlimit was using "%lu" in a lot of printf format specifiers to print > 64-bit integers. This is incorrect on 32-bit architectures because > "long int" is 32-bits there. On MIPS, it was causing iptables to > segfault when printing these integers. > > Fix by using the correct format specifier. > > Signed-off-by: James Cowgill <James.Cowgill@imgtec.com> > --- > extensions/libxt_hashlimit.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > For reference, this is related to debian bug #859775 [0] [0] https://bugs.debian.org/859775 -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Apr 07, 2017 at 12:47:29PM +0100, James Cowgill wrote: > hashlimit was using "%lu" in a lot of printf format specifiers to print > 64-bit integers. This is incorrect on 32-bit architectures because > "long int" is 32-bits there. On MIPS, it was causing iptables to > segfault when printing these integers. > > Fix by using the correct format specifier. One comment below... > @@ -262,7 +262,7 @@ static uint64_t parse_burst(const char *burst, int revision) > if (v > max) > xtables_error(PARAMETER_PROBLEM, "bad value for option " > "\"--hashlimit-burst\", value \"%s\" too large " > - "(max %lumb).", burst, max/1024/1024); > + "(max %" PRIu64 "mb).", burst, max/1024/1024); ^ ^ I can remove this whitespaces, right? I'm looking at other spots in the iptables code and I would like to keep this consistent. No need to resent the patch, I can just mangle this here and apply. Thanks! -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/04/17 21:29, Pablo Neira Ayuso wrote: > On Fri, Apr 07, 2017 at 12:47:29PM +0100, James Cowgill wrote: >> hashlimit was using "%lu" in a lot of printf format specifiers to print >> 64-bit integers. This is incorrect on 32-bit architectures because >> "long int" is 32-bits there. On MIPS, it was causing iptables to >> segfault when printing these integers. >> >> Fix by using the correct format specifier. > > One comment below... > >> @@ -262,7 +262,7 @@ static uint64_t parse_burst(const char *burst, int revision) >> if (v > max) >> xtables_error(PARAMETER_PROBLEM, "bad value for option " >> "\"--hashlimit-burst\", value \"%s\" too large " >> - "(max %lumb).", burst, max/1024/1024); >> + "(max %" PRIu64 "mb).", burst, max/1024/1024); > ^ ^ > > I can remove this whitespaces, right? I'm looking at other spots in > the iptables code and I would like to keep this consistent. > > No need to resent the patch, I can just mangle this here and apply. Yes removing the spaces around PRIu64 should be fine. James -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Saturday 2017-04-08 22:29, Pablo Neira Ayuso wrote: >> @@ -262,7 +262,7 @@ static uint64_t parse_burst(const char *burst, int revision) >> if (v > max) >> xtables_error(PARAMETER_PROBLEM, "bad value for option " >> "\"--hashlimit-burst\", value \"%s\" too large " >> - "(max %lumb).", burst, max/1024/1024); >> + "(max %" PRIu64 "mb).", burst, max/1024/1024); > ^ ^ > >I can remove this whitespaces, right? With my distro hat on: Clumping these together like "foo"BAR"baz" has already caused compile failures in the broader scope of distributions (thousands of packages) because languages introduced new tokenization rules. Admittedly, this occurred in C++ (namely, user-defined string literals), but it does show that tokens which logically are separate should stay separate. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Apr 10, 2017 at 05:59:07PM +0200, Jan Engelhardt wrote: > > On Saturday 2017-04-08 22:29, Pablo Neira Ayuso wrote: > >> @@ -262,7 +262,7 @@ static uint64_t parse_burst(const char *burst, int revision) > >> if (v > max) > >> xtables_error(PARAMETER_PROBLEM, "bad value for option " > >> "\"--hashlimit-burst\", value \"%s\" too large " > >> - "(max %lumb).", burst, max/1024/1024); > >> + "(max %" PRIu64 "mb).", burst, max/1024/1024); > > ^ ^ > > > >I can remove this whitespaces, right? > > With my distro hat on: > > Clumping these together like "foo"BAR"baz" has already caused compile failures > in the broader scope of distributions (thousands of packages) because languages > introduced new tokenization rules. Admittedly, this occurred in C++ (namely, > user-defined string literals), but it does show that tokens which logically are > separate should stay separate. Existing code doesn't add space, so I would like we don't introduce inconsistencies. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/extensions/libxt_hashlimit.c b/extensions/libxt_hashlimit.c index 52fc4fa..63cf903 100644 --- a/extensions/libxt_hashlimit.c +++ b/extensions/libxt_hashlimit.c @@ -12,9 +12,9 @@ */ #define _BSD_SOURCE 1 #define _ISOC99_SOURCE 1 +#include <inttypes.h> #include <math.h> #include <stdbool.h> -#include <stdint.h> #include <stdio.h> #include <string.h> #include <stdlib.h> @@ -262,7 +262,7 @@ static uint64_t parse_burst(const char *burst, int revision) if (v > max) xtables_error(PARAMETER_PROBLEM, "bad value for option " "\"--hashlimit-burst\", value \"%s\" too large " - "(max %lumb).", burst, max/1024/1024); + "(max %" PRIu64 "mb).", burst, max/1024/1024); return v; } @@ -285,8 +285,8 @@ static bool parse_bytes(const char *rate, void *val, struct hashlimit_mt_udata * tmp = (uint64_t) r * factor; if (tmp > max) xtables_error(PARAMETER_PROBLEM, - "Rate value too large \"%llu\" (max %lu)\n", - (unsigned long long)tmp, max); + "Rate value too large \"%" PRIu64 "\" (max %" PRIu64 ")\n", + tmp, max); tmp = bytes_to_cost(tmp); if (tmp == 0) @@ -557,7 +557,7 @@ static void hashlimit_mt_check_v1(struct xt_fcheck_call *cb) if (cb->xflags & F_BURST) { if (info->cfg.burst < cost_to_bytes(info->cfg.avg)) xtables_error(PARAMETER_PROBLEM, - "burst cannot be smaller than %lub", cost_to_bytes(info->cfg.avg)); + "burst cannot be smaller than %" PRIu64 "b", cost_to_bytes(info->cfg.avg)); burst = info->cfg.burst; burst /= cost_to_bytes(info->cfg.avg); @@ -587,7 +587,7 @@ static void hashlimit_mt_check(struct xt_fcheck_call *cb) if (cb->xflags & F_BURST) { if (info->cfg.burst < cost_to_bytes(info->cfg.avg)) xtables_error(PARAMETER_PROBLEM, - "burst cannot be smaller than %lub", cost_to_bytes(info->cfg.avg)); + "burst cannot be smaller than %" PRIu64 "b", cost_to_bytes(info->cfg.avg)); burst = info->cfg.burst; burst /= cost_to_bytes(info->cfg.avg); @@ -631,7 +631,7 @@ static uint32_t print_rate(uint32_t period, int revision) || _rates[i].mult/period < _rates[i].mult%period) break; - printf(" %lu/%s", _rates[i-1].mult / period, _rates[i-1].name); + printf(" %" PRIu64 "/%s", _rates[i-1].mult / period, _rates[i-1].name); /* return in msec */ return _rates[i-1].mult / scale * 1000; }
hashlimit was using "%lu" in a lot of printf format specifiers to print 64-bit integers. This is incorrect on 32-bit architectures because "long int" is 32-bits there. On MIPS, it was causing iptables to segfault when printing these integers. Fix by using the correct format specifier. Signed-off-by: James Cowgill <James.Cowgill@imgtec.com> --- extensions/libxt_hashlimit.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)