| Message ID | 1268094699.1617.5.camel@Joe-Laptop.home |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
On Mon, Mar 08, 2010 at 04:31:39PM -0800, Joe Perches wrote: > NIPQUAD has very few uses left. > > Remove this use and make the code have the identical form of the only > other use of "%u,%u,%u,%u,%u,%u" in net/ipv4/netfilter/nf_nat_ftp.c > > Signed-off-by: Joe Perches <joe@perches.com> Acked-by: Simon Horman <horms@verge.net.au> > --- > net/netfilter/ipvs/ip_vs_ftp.c | 10 ++++++++-- > 1 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/net/netfilter/ipvs/ip_vs_ftp.c > b/net/netfilter/ipvs/ip_vs_ftp.c > index 73f38ea..9f63283 100644 > --- a/net/netfilter/ipvs/ip_vs_ftp.c > +++ b/net/netfilter/ipvs/ip_vs_ftp.c > @@ -208,8 +208,14 @@ static int ip_vs_ftp_out(struct ip_vs_app *app, > struct ip_vs_conn *cp, > */ > from.ip = n_cp->vaddr.ip; > port = n_cp->vport; > - sprintf(buf, "%u,%u,%u,%u,%u,%u", NIPQUAD(from.ip), > - (ntohs(port)>>8)&255, ntohs(port)&255); > + snprintf(buf, sizeof(buf), "%u,%u,%u,%u,%u,%u", > + ((unsigned char *)&from.ip)[0], > + ((unsigned char *)&from.ip)[1], > + ((unsigned char *)&from.ip)[2], > + ((unsigned char *)&from.ip)[3], > + ntohs(port) >> 8, > + ntohs(port) & 0xFF); > + > buf_len = strlen(buf); > > /* > > > -- > To unsubscribe from this list: send the line "unsubscribe lvs-devel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Joe Perches wrote: > NIPQUAD has very few uses left. > > Remove this use and make the code have the identical form of the only > other use of "%u,%u,%u,%u,%u,%u" in net/ipv4/netfilter/nf_nat_ftp.c Are we trying to remove NIPQUAD? In my opinion the current code is preferrable to open-coding NIPQUAD. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2010-03-15 at 17:20 +0100, Patrick McHardy wrote: > Joe Perches wrote: > > NIPQUAD has very few uses left. > > Remove this use and make the code have the identical form of the only > > other use of "%u,%u,%u,%u,%u,%u" in net/ipv4/netfilter/nf_nat_ftp.c > > Are we trying to remove NIPQUAD? That is a goal for me yes. Elimination of unnecessary stuff from kernel.h is useful. > In my opinion the current code is > preferrable to open-coding NIPQUAD. $ grep -r --include=*.[ch] NIPQUAD * include/linux/kernel.h:#define NIPQUAD(addr) \ include/linux/kernel.h:#define NIPQUAD_FMT "%u.%u.%u.%u" net/netfilter/ipvs/ip_vs_ftp.c: sprintf(buf, "%u,%u,%u,%u,%u,%u", NIPQUAD(from.ip), As of right now NIPQUAD_FMT is unused and could be removed from kernel.h There's 1 use of NIPQUAD. Maybe instead of removal the #define could be moved to netfilter.h or netfilter_ipv4.h Another is to consolidate the two uses of "%u,%u,%u,%u,%u,%u" $ grep -r --include=*.[ch] "%u,%u,%u,%u,%u,u%" * fs/cachefiles/bind.c: _enter("{%u,%u,%u,%u,%u,%u},%s", net/ipv4/netfilter/nf_nat_ftp.c: return snprintf(buffer, buflen, "%u,%u,%u,%u,%u,%u", net/netfilter/ipvs/ip_vs_ftp.c: sprintf(buf, "%u,%u,%u,%u,%u,%u", NIPQUAD(from.ip), Maybe these 2 open coded sprintf/snprintf could be consolidated and moved to an appropriate include like netfilter_ipv4 or another. What do you suggest? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Joe Perches wrote: > On Mon, 2010-03-15 at 17:20 +0100, Patrick McHardy wrote: >> Joe Perches wrote: >>> NIPQUAD has very few uses left. >>> Remove this use and make the code have the identical form of the only >>> other use of "%u,%u,%u,%u,%u,%u" in net/ipv4/netfilter/nf_nat_ftp.c >> Are we trying to remove NIPQUAD? > > That is a goal for me yes. > Elimination of unnecessary stuff from kernel.h is useful. > >> In my opinion the current code is >> preferrable to open-coding NIPQUAD. > > $ grep -r --include=*.[ch] NIPQUAD * > include/linux/kernel.h:#define NIPQUAD(addr) \ > include/linux/kernel.h:#define NIPQUAD_FMT "%u.%u.%u.%u" > net/netfilter/ipvs/ip_vs_ftp.c: sprintf(buf, "%u,%u,%u,%u,%u,%u", NIPQUAD(from.ip), > > As of right now NIPQUAD_FMT is unused and > could be removed from kernel.h > > There's 1 use of NIPQUAD. > > Maybe instead of removal the #define could be > moved to netfilter.h or netfilter_ipv4.h > > Another is to consolidate the two uses of > "%u,%u,%u,%u,%u,%u" > > $ grep -r --include=*.[ch] "%u,%u,%u,%u,%u,u%" * > fs/cachefiles/bind.c: _enter("{%u,%u,%u,%u,%u,%u},%s", > net/ipv4/netfilter/nf_nat_ftp.c: return snprintf(buffer, buflen, "%u,%u,%u,%u,%u,%u", > net/netfilter/ipvs/ip_vs_ftp.c: sprintf(buf, "%u,%u,%u,%u,%u,%u", NIPQUAD(from.ip), > > Maybe these 2 open coded sprintf/snprintf could be > consolidated and moved to an appropriate include > like netfilter_ipv4 or another. > > What do you suggest? Well, removing the last user sounds acceptable :) Applied, thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Patrick McHardy <kaber@trash.net> Date: Mon, 15 Mar 2010 17:20:19 +0100 > Joe Perches wrote: >> NIPQUAD has very few uses left. >> >> Remove this use and make the code have the identical form of the only >> other use of "%u,%u,%u,%u,%u,%u" in net/ipv4/netfilter/nf_nat_ftp.c > > Are we trying to remove NIPQUAD? Yes. -- To unsubscribe from this list: send the line "unsubscribe netdev" 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/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c index 73f38ea..9f63283 100644 --- a/net/netfilter/ipvs/ip_vs_ftp.c +++ b/net/netfilter/ipvs/ip_vs_ftp.c @@ -208,8 +208,14 @@ static int ip_vs_ftp_out(struct ip_vs_app *app, struct ip_vs_conn *cp, */ from.ip = n_cp->vaddr.ip; port = n_cp->vport; - sprintf(buf, "%u,%u,%u,%u,%u,%u", NIPQUAD(from.ip), - (ntohs(port)>>8)&255, ntohs(port)&255); + snprintf(buf, sizeof(buf), "%u,%u,%u,%u,%u,%u", + ((unsigned char *)&from.ip)[0], + ((unsigned char *)&from.ip)[1], + ((unsigned char *)&from.ip)[2], + ((unsigned char *)&from.ip)[3], + ntohs(port) >> 8, + ntohs(port) & 0xFF); + buf_len = strlen(buf);
NIPQUAD has very few uses left. Remove this use and make the code have the identical form of the only other use of "%u,%u,%u,%u,%u,%u" in net/ipv4/netfilter/nf_nat_ftp.c Signed-off-by: Joe Perches <joe@perches.com> --- net/netfilter/ipvs/ip_vs_ftp.c | 10 ++++++++-- 1 files changed, 8 insertions(+), 2 deletions(-) /* -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html