| Message ID | 1357534195.21481.31.camel@joe-AO722 |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
于 2013年01月07日 12:49, Joe Perches 写道: > On Sun, 2013-01-06 at 19:19 -0800, Joe Perches wrote: >> Maybe these days this should be another vsprintf %p extension >> like %pM when the DECLARE_MAC_BUF/print_mac uses were converted. >> >> (or maybe extend %ph for ssids with %*phs, length, array) > excuse me: I do not quite know how to reply the [RFC PATCH]. it would be better if you can tell me how to do for [RFC PATCH], thanks. :-) at least for me: although it is good idea to add common, widely used features in public tools function. after searching the relative source code: it seems print ssid is not quite widely used (although it is a common feature). it is used by drivers/net/wireless/libertas it is used by drivers/net/wireless/ipw2x00 no additional using in current kernel source code wide. if another modules want to print ssid, they can still call print_ssid now (EXPORT_SYMBOL in net/wireless). so at least now, it is not necessary to add this feature to public tools function. Regards gchen. > Maybe like this: > > lib/vsprintf.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 55 insertions(+), 2 deletions(-) > > diff --git a/lib/vsprintf.c b/lib/vsprintf.c > index fab33a9..98916a0 100644 > --- a/lib/vsprintf.c > +++ b/lib/vsprintf.c > @@ -26,6 +26,7 @@ > #include <linux/math64.h> > #include <linux/uaccess.h> > #include <linux/ioport.h> > +#include <linux/ieee80211.h> > #include <net/addrconf.h> > > #include <asm/page.h> /* for PAGE_SIZE */ > @@ -660,10 +661,59 @@ char *resource_string(char *buf, char *end, struct resource *res, > } > > static noinline_for_stack > +char *ssid_string(char *buf, char *end, u8 *addr, struct printf_spec spec, > + const char *fmt) > +{ > + int i, len = 1; /* if we pass %*p, field width remains > + negative value, fallback to the default */ > + > + if (spec.field_width == 0) > + /* nothing to print */ > + return buf; > + > + if (ZERO_OR_NULL_PTR(addr)) > + /* NULL pointer */ > + return string(buf, end, NULL, spec); > + > + if (spec.field_width > 0) > + len = min_t(int, spec.field_width, IEEE80211_MAX_SSID_LEN); > + > + for (i = 0; i < len && buf < end; i++) { > + if (isprint(addr[i])) { > + *buf++ = addr[i]; > + continue; > + } > + *buf++ = '\\'; > + if (buf >= end) > + continue; > + if (addr[i] == '\0') > + *buf++ = '0'; > + else if (addr[i] == '\n') > + *buf++ = 'n'; > + else if (addr[i] == '\r') > + *buf++ = 'r'; > + else if (addr[i] == '\t') > + *buf++ = 't'; > + else if (addr[i] == '\\') > + *buf++ = '\\'; > + else { > + if (buf < end) > + *buf++ = ((addr[i] >> 6) & 7) + '0'; > + if (buf < end) > + *buf++ = ((addr[i] >> 3) & 7) + '0'; > + if (buf < end) > + *buf++ = ((addr[i] >> 0) & 7) + '0'; > + } > + } > + > + return buf; > +} > + > +static noinline_for_stack > char *hex_string(char *buf, char *end, u8 *addr, struct printf_spec spec, > const char *fmt) > { > - int i, len = 1; /* if we pass '%ph[CDN]', field witdh remains > + int i, len = 1; /* if we pass '%ph[CDN]', field width remains > negative value, fallback to the default */ > char separator; > > @@ -695,7 +745,6 @@ char *hex_string(char *buf, char *end, u8 *addr, struct printf_spec spec, > > for (i = 0; i < len && buf < end - 1; i++) { > buf = hex_byte_pack(buf, addr[i]); > - > if (buf < end && separator && i != len - 1) > *buf++ = separator; > } > @@ -1016,6 +1065,8 @@ int kptr_restrict __read_mostly; > * [0][1][2][3]-[4][5]-[6][7]-[8][9]-[10][11][12][13][14][15] > * little endian output byte order is: > * [3][2][1][0]-[5][4]-[7][6]-[8][9]-[10][11][12][13][14][15] > + * - 'D' For a 80211 SSID, it prints the SSID escaping any non-printable > + characters with a leading \ and octal or [0nrt\] > * - 'V' For a struct va_format which contains a format string * and va_list *, > * call vsnprintf(->format, *->va_list). > * Implements a "recursive vsnprintf". > @@ -1088,6 +1139,8 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr, > break; > case 'U': > return uuid_string(buf, end, ptr, spec, fmt); > + case 'D': > + return ssid_string(buf, end, ptr, spec, fmt); > case 'V': > { > va_list va; > > > >
On Mon, 2013-01-07 at 14:07 +0800, Chen Gang wrote: > 于 2013年01月07日 12:49, Joe Perches 写道: > > On Sun, 2013-01-06 at 19:19 -0800, Joe Perches wrote: > >> Maybe these days this should be another vsprintf %p extension > >> like %pM when the DECLARE_MAC_BUF/print_mac uses were converted. > >> > >> (or maybe extend %ph for ssids with %*phs, length, array) > > > excuse me: > I do not quite know how to reply the [RFC PATCH]. > it would be better if you can tell me how to do for [RFC PATCH], thanks. You did fine except you unnecessarily quoted the entire original email. Remember to trim your replies please. _lots_ of people read these mailing lists and unnecessary quoting wastes all of their times. > at least for me: > although it is good idea to add common, widely used features in public tools function. It's akin to most of the minor uuid/bluetooth mac, etc codes in vsprintf I've added. cheers, Joe -- 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
于 2013年01月07日 14:37, Joe Perches 写道: > You did fine except you unnecessarily quoted the entire original email. > Remember to trim your replies please. _lots_ of people read these > mailing lists and unnecessary quoting wastes all of their times. > ok, thank you. I should notice, next time. :-) >> > at least for me: >> > although it is good idea to add common, widely used features in public tools function. > It's akin to most of the minor uuid/bluetooth mac, etc codes after see the comments for function pointer in lib/vsprintf.c sorry for I do not think it is suitable to add ssid in vsprintf all formate features in vsprintf are face to kernel wide, not for one sub system one exception is for mac and ip: they have already been well known in kernel wide every one knows about mac and ip. I feel, our ssid is not quite well known in kernel wide maybe current place (net/wireless/) is not quite suitable for print_ssid: for bluetooth, it is not belong to net/wireless. if necessary, I suggest to reconstruct it within net sub system wide, not in kernel wide. (it is also my fault for my original reply, it is not quite clear) Regards
On Sun, 2013-01-06 at 20:49 -0800, Joe Perches wrote: > On Sun, 2013-01-06 at 19:19 -0800, Joe Perches wrote: > > Maybe these days this should be another vsprintf %p extension > > like %pM when the DECLARE_MAC_BUF/print_mac uses were converted. > > > > (or maybe extend %ph for ssids with %*phs, length, array) > > Maybe like this: > + * - 'D' For a 80211 SSID, it prints the SSID escaping any non-printable > + characters with a leading \ and octal or [0nrt\] Honestly, I'm not sure it's worth it. print_ssid() is used in two or three legacy drivers only, not in any modern driver, and is unlikely to be used in the more modern drivers due to tracing etc. johannes -- 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, 2013-01-07 at 08:47 +0100, Johannes Berg wrote: > On Sun, 2013-01-06 at 20:49 -0800, Joe Perches wrote: > > On Sun, 2013-01-06 at 19:19 -0800, Joe Perches wrote: > > > Maybe these days this should be another vsprintf %p extension > > > like %pM when the DECLARE_MAC_BUF/print_mac uses were converted. > > > > > > (or maybe extend %ph for ssids with %*phs, length, array) > > > > Maybe like this: > > > + * - 'D' For a 80211 SSID, it prints the SSID escaping any non-printable > > + characters with a leading \ and octal or [0nrt\] > > Honestly, I'm not sure it's worth it. Neither am I really, the only value I see in it is the reduction in stack consumption by 100 bytes or so per use. > print_ssid() is used in two or > three legacy drivers only, not in any modern driver, and is unlikely to > be used in the more modern drivers due to tracing etc. Swell. It was just another way to correct those overrun errors Chen Gang found. -- 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
于 2013年01月08日 01:22, Joe Perches 写道: > On Mon, 2013-01-07 at 08:47 +0100, Johannes Berg wrote: >> > print_ssid() is used in two or >> > three legacy drivers only, not in any modern driver, and is unlikely to >> > be used in the more modern drivers due to tracing etc. > Swell. It was just another way to correct those overrun > errors Chen Gang found. > sorry, I am not quite clear about what you said.
On Tue, 2013-01-08 at 10:57 +0800, Chen Gang wrote: > 于 2013年01月08日 01:22, Joe Perches 写道: > > On Mon, 2013-01-07 at 08:47 +0100, Johannes Berg wrote: > >> > print_ssid() is used in two or > >> > three legacy drivers only, not in any modern driver, and is unlikely to > >> > be used in the more modern drivers due to tracing etc. > > Swell. It was just another way to correct those overrun > > errors Chen Gang found. > > > sorry, I am not quite clear about what you said. You found some overrun errors and proposed a patch. Your solution could output incomplete SSIDs. I proposed a different patch that would fully output any binary/non-ascii printable SSID. I also proposed a different mechanism to avoid the overrun via printk and also avoid possibly excessive stack consumption as a means for John Linville to select 1 of the above options. I don't care what is done with any of those proposed patches. Clear? -- 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
于 2013年01月08日 11:11, Joe Perches 写道: > On Tue, 2013-01-08 at 10:57 +0800, Chen Gang wrote: >> sorry, I am not quite clear about what you said. > > You found some overrun errors and proposed a patch. > Your solution could output incomplete SSIDs. > > I proposed a different patch that would fully output > any binary/non-ascii printable SSID. > > I also proposed a different mechanism to avoid the > overrun via printk and also avoid possibly excessive > stack consumption as a means for John Linville to > select 1 of the above options. > > I don't care what is done with any of those proposed > patches. > > Clear? > ok
diff --git a/lib/vsprintf.c b/lib/vsprintf.c index fab33a9..98916a0 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -26,6 +26,7 @@ #include <linux/math64.h> #include <linux/uaccess.h> #include <linux/ioport.h> +#include <linux/ieee80211.h> #include <net/addrconf.h> #include <asm/page.h> /* for PAGE_SIZE */ @@ -660,10 +661,59 @@ char *resource_string(char *buf, char *end, struct resource *res, } static noinline_for_stack +char *ssid_string(char *buf, char *end, u8 *addr, struct printf_spec spec, + const char *fmt) +{ + int i, len = 1; /* if we pass %*p, field width remains + negative value, fallback to the default */ + + if (spec.field_width == 0) + /* nothing to print */ + return buf; + + if (ZERO_OR_NULL_PTR(addr)) + /* NULL pointer */ + return string(buf, end, NULL, spec); + + if (spec.field_width > 0) + len = min_t(int, spec.field_width, IEEE80211_MAX_SSID_LEN); + + for (i = 0; i < len && buf < end; i++) { + if (isprint(addr[i])) { + *buf++ = addr[i]; + continue; + } + *buf++ = '\\'; + if (buf >= end) + continue; + if (addr[i] == '\0') + *buf++ = '0'; + else if (addr[i] == '\n') + *buf++ = 'n'; + else if (addr[i] == '\r') + *buf++ = 'r'; + else if (addr[i] == '\t') + *buf++ = 't'; + else if (addr[i] == '\\') + *buf++ = '\\'; + else { + if (buf < end) + *buf++ = ((addr[i] >> 6) & 7) + '0'; + if (buf < end) + *buf++ = ((addr[i] >> 3) & 7) + '0'; + if (buf < end) + *buf++ = ((addr[i] >> 0) & 7) + '0'; + } + } + + return buf; +} + +static noinline_for_stack char *hex_string(char *buf, char *end, u8 *addr, struct printf_spec spec, const char *fmt) { - int i, len = 1; /* if we pass '%ph[CDN]', field witdh remains + int i, len = 1; /* if we pass '%ph[CDN]', field width remains negative value, fallback to the default */ char separator; @@ -695,7 +745,6 @@ char *hex_string(char *buf, char *end, u8 *addr, struct printf_spec spec, for (i = 0; i < len && buf < end - 1; i++) { buf = hex_byte_pack(buf, addr[i]); - if (buf < end && separator && i != len - 1) *buf++ = separator; } @@ -1016,6 +1065,8 @@ int kptr_restrict __read_mostly; * [0][1][2][3]-[4][5]-[6][7]-[8][9]-[10][11][12][13][14][15] * little endian output byte order is: * [3][2][1][0]-[5][4]-[7][6]-[8][9]-[10][11][12][13][14][15] + * - 'D' For a 80211 SSID, it prints the SSID escaping any non-printable + characters with a leading \ and octal or [0nrt\] * - 'V' For a struct va_format which contains a format string * and va_list *, * call vsnprintf(->format, *->va_list). * Implements a "recursive vsnprintf". @@ -1088,6 +1139,8 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr, break; case 'U': return uuid_string(buf, end, ptr, spec, fmt); + case 'D': + return ssid_string(buf, end, ptr, spec, fmt); case 'V': { va_list va;