Hi Ira, On 14:44 Wed 11 Mar , Ira Weiny wrote: > > From: Ira Weiny <[email protected]> > Date: Wed, 11 Mar 2009 10:45:25 -0700 > Subject: [PATCH] Update mad formatting functions. > > Add mad_snprintf w/ man page > Add mad_fprintf w/ man page > Add comments to document current functions. > Rename parameters to avoid confusion with other functions which take > "buf" > Mark mad_print_field as deprecated > > Signed-off-by: Ira Weiny <[email protected]>
Nice stuff! I have some implementation comments below. > --- > libibmad/Makefile.am | 2 + > libibmad/include/infiniband/mad.h | 28 +++- > libibmad/man/mad_fprintf.3 | 82 ++++++++++ > libibmad/man/mad_snprintf.3 | 2 + > libibmad/src/fields.c | 319 > ++++++++++++++++++++++++++++++++++++- > libibmad/src/libibmad.map | 2 + > 6 files changed, 428 insertions(+), 7 deletions(-) > create mode 100644 libibmad/man/mad_fprintf.3 > create mode 100644 libibmad/man/mad_snprintf.3 > > diff --git a/libibmad/Makefile.am b/libibmad/Makefile.am > index 4f3ba98..da32899 100644 > --- a/libibmad/Makefile.am > +++ b/libibmad/Makefile.am > @@ -5,6 +5,8 @@ INCLUDES = -I$(srcdir)/include -I$(includedir) > > lib_LTLIBRARIES = libibmad.la > > +man_MANS = man/mad_fprintf.3 man/mad_snprintf.3 > + > libibmad_la_CFLAGS = -Wall > > if HAVE_LD_VERSION_SCRIPT > diff --git a/libibmad/include/infiniband/mad.h > b/libibmad/include/infiniband/mad.h > index 064cbb7..897b91b 100644 > --- a/libibmad/include/infiniband/mad.h > +++ b/libibmad/include/infiniband/mad.h > @@ -719,9 +719,31 @@ MAD_EXPORT void mad_set_array(void *buf, int base_offs, > enum MAD_FIELDS field, v > MAD_EXPORT void mad_get_array(void *buf, int base_offs, enum MAD_FIELDS > field, void *val); > MAD_EXPORT void mad_decode_field(uint8_t * buf, enum MAD_FIELDS field, void > *val); > MAD_EXPORT void mad_encode_field(uint8_t * buf, enum MAD_FIELDS field, void > *val); > -MAD_EXPORT int mad_print_field(enum MAD_FIELDS field, const char *name, void > *val); > -MAD_EXPORT char *mad_dump_field(enum MAD_FIELDS field, char *buf, int bufsz, > void *val); > -MAD_EXPORT char *mad_dump_val(enum MAD_FIELDS field, char *buf, int bufsz, > void *val); > +MAD_EXPORT int mad_print_field(enum MAD_FIELDS field, const char *name, void > *val) > + DEPRECATED; > + > +/** > + * The following functions print fields to "s" in various ways > + * > + * mad_dump_[val|field] take a value "val" and use "field" to format it > + * > + * mad_snprint_field takes a data buffer "buf" and uses field to extract and > + * format it. > + * > + * RETURN "s" or NULL on failure > + */ > +MAD_EXPORT char *mad_dump_field(enum MAD_FIELDS field, char *s, int n, void > *val); > + /* outputs string "<field>:........<val>" */ > +MAD_EXPORT char *mad_dump_val(enum MAD_FIELDS field, char *s, int n, void > *val); > + /* outputs string "<val>" */ > + > +/** > + * printf functions > + * input's "standard" printf parameters except for "buf" which is a mad > buffer > + * return the number of actual chars written to "s" or "stream" > + */ > +MAD_EXPORT int mad_snprintf(char *s, size_t n, uint8_t *buf, const char > *format, ...); > +MAD_EXPORT int mad_fprintf(FILE *stream, uint8_t *buf, const char *format, > ...); > > /* mad.c */ > MAD_EXPORT void *mad_encode(void *buf, ib_rpc_t * rpc, ib_dr_path_t * drpath, > diff --git a/libibmad/man/mad_fprintf.3 b/libibmad/man/mad_fprintf.3 > new file mode 100644 > index 0000000..e69bd44 > --- /dev/null > +++ b/libibmad/man/mad_fprintf.3 > @@ -0,0 +1,82 @@ > +.\" -*- nroff -*- > +.\" > +.TH MAD_FPRINTF 3 "Feb 26, 2009" "OpenIB" "OpenIB Programmer\'s Manual" > +.SH "NAME" > +mad_fprintf, mad_snprintf \- formatted output conversion for mad packets > +.SH "SYNOPSIS" > +.nf > +.B #include <infiniband/mad.h> > +.sp > +.BI "MAD_EXPORT int mad_snprintf(char " "*s" ", size_t "n ", uint8_t " > "*buf" ", const char " "*format" ", ...); > +.BI "MAD_EXPORT int mad_fprintf(FILE " "*stream" ", uint8_t " "*buf" ", > const char " "*format" ", ...); > +.fi > +.SH "DESCRIPTION" > +Similar to the printf family of functions. The exception being they do > +.B not > +accept all conversion specifiers and they accept a "buf" parameter which > +represents a mad data buffer. This buffer is used to extract and print > fields > +as specified with the > +.B %F > +format specifier. > +.PP > +The following conversion specifiers are > +.B not > +supported. > +.B e, E, f, g, G, a, A, C, S, m > +and > +.B n > +.PP > +.B F > +The %F specifier is used to print out fields decoded from the "buf" data > +buffer. ib_mad_f table also has a name string field. I think it can be useful too - will help to unify outputs. Of course this can be done as subsequent patch. > +.I enum MAD_FIELDS\fR > +values should be used to specify the field to be decoded. > +.PP > +.SH "EXAMPLES" > +.nf > +char portinfo[64]; > +void *pi = portinfo; > +.PP > +if (!smp_query(pi, portid, IB_ATTR_PORT_INFO, portnum, timeout)) > +.in +8 > + return -1; > +.in -16 > +.PP > +mad_fprintf(stdout, pi, "Port info (%s):\\n" > +.in +16 > +" %-10s: %F\\n" > +" %-10s: %F\\n" > +" %-10s: %F\\n" > +" %-10s: %F\\n" > +" %-10s: %F\\n" > +" %-10s: %F\\n", > +portid2str(portid), > +"LID", IB_PORT_LID_F, > +"LMC", IB_PORT_LMC_F, > +"state", IB_PORT_STATE_F, > +"physstate", IB_PORT_PHYS_STATE_F, > +"linkwidth", IB_PORT_LINK_WIDTH_ACTIVE_F, > +"linkspeed", IB_PORT_LINK_SPEED_ACTIVE_F > +); > +.in -16 > +.PP > +Results in the output. > +.PP > +Port info (DR path slid 0; dlid 0; 0,1,14): > +.in +3 > +LID : 0x0016 Lids are printed as decimals. > +LMC : 0 > +state : Active > +physstate : LinkUp > +linkwidth : 4X > +linkspeed : 5.0 Gbps > +.in -3 > + > +.SH "RETURN VALUE" > +.B return the number of characters printed. > + > +.SH "SEE ALSO" > +.BR printf (3) > +.SH "AUTHOR" > +.TP > +Ira Weiny <[email protected]> > diff --git a/libibmad/man/mad_snprintf.3 b/libibmad/man/mad_snprintf.3 > new file mode 100644 > index 0000000..c004ab9 > --- /dev/null > +++ b/libibmad/man/mad_snprintf.3 > @@ -0,0 +1,2 @@ > +.TH MAD_SNPRINTF 3 "Feb 26, 2009" "OpenIB" "OpenIB Programmer\'s Manual" > +.so man3/mad_fprintf.3 > diff --git a/libibmad/src/fields.c b/libibmad/src/fields.c > index 19c8fc1..acb1180 100644 > --- a/libibmad/src/fields.c > +++ b/libibmad/src/fields.c > @@ -38,7 +38,9 @@ > > #include <stdio.h> > #include <stdlib.h> > +#define _GNU_SOURCE Where is _GNU_SOURCE really used (I didn't find)? > #include <string.h> > +#include <stdarg.h> > > #include <infiniband/mad.h> > > @@ -442,6 +444,9 @@ static const ib_field_t ib_mad_f[] = { > > }; > > +#define MAD_FIELD_MAX_BYTE_LEN (256) > + /* currently "Vendor2Data" increased to the next power of 2 */ > + > static void _set_field64(void *buf, int base_offs, const ib_field_t * f, > uint64_t val) > { > @@ -666,6 +671,7 @@ static int _mad_print_field(const ib_field_t * f, const > char *name, void *val, > valsz ? valsz : ALIGN(f->bitlen, 8) / 8); > } > > +/* This function is deprecated use mad_snprint_field or mad_dump_* instead */ > int mad_print_field(enum MAD_FIELDS field, const char *name, void *val) > { > if (field <= IB_NO_FIELD || field >= IB_FIELD_LAST_) > @@ -673,16 +679,321 @@ int mad_print_field(enum MAD_FIELDS field, const char > *name, void *val) > return _mad_print_field(ib_mad_f + field, name, val, 0); > } > > -char *mad_dump_field(enum MAD_FIELDS field, char *buf, int bufsz, void *val) > +char *mad_dump_field(enum MAD_FIELDS field, char *s, int n, void *val) > { > if (field <= IB_NO_FIELD || field >= IB_FIELD_LAST_) > return 0; > - return _mad_dump_field(ib_mad_f + field, 0, buf, bufsz, val); > + return _mad_dump_field(ib_mad_f + field, 0, s, n, val); > } > > -char *mad_dump_val(enum MAD_FIELDS field, char *buf, int bufsz, void *val) > +char *mad_dump_val(enum MAD_FIELDS field, char *s, int n, void *val) > { > if (field <= IB_NO_FIELD || field >= IB_FIELD_LAST_) > return 0; > - return _mad_dump_val(ib_mad_f + field, buf, bufsz, val); > + return _mad_dump_val(ib_mad_f + field, s, n, val); > } > + > +#define ZEROPAD 1 /* pad with zero */ > +#define SIGN 2 /* unsigned/signed long */ > +#define PLUS 4 /* show plus */ > +#define SPACE 8 /* space if plus */ > +#define LEFT 16 /* left justified */ > +#define SPECIAL 32 /* 0x */ > +#define LARGE 64 /* use 'ABCDEF' instead of 'abcdef' */ > + > +static char * number(char * str, size_t n, int *rc, > + unsigned long long num, int base, int size, > + int precision, int type) > +{ > + char c,sign,tmp[66]; > + const char *digits="0123456789abcdefghijklmnopqrstuvwxyz"; > + int i; > + > +/* Macro allows for checking length > + * remove 1 to allow for \0 char */ > +#define WRITE_CHAR_RET(c) do { \ > + *str++ = c; \ > + if (++(*rc) >= (n-1)) { \ > + return (str); \ > + } \ > +} while(0) > + > + if (type & LARGE) > + digits = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ"; > + if (type & LEFT) > + type &= ~ZEROPAD; > + if (base < 2 || base > 36) > + return 0; > + c = (type & ZEROPAD) ? '0' : ' '; > + sign = 0; > + if (type & SIGN) { > + if ((signed long long)num < 0) { > + sign = '-'; > + num = - (signed long long)num; > + size--; > + } else if (type & PLUS) { > + sign = '+'; > + size--; > + } else if (type & SPACE) { > + sign = ' '; > + size--; > + } > + } > + if (type & SPECIAL) { > + if (base == 16) > + size -= 2; > + else if (base == 8) > + size--; > + } > + i = 0; > + if (num == 0) > + tmp[i++]='0'; > + else { > + while (num >= base) { > + tmp[i++] = digits[num % base]; > + num /= base; > + } > + tmp[i++] = digits[num]; > + } > + if (i > precision) > + precision = i; > + size -= precision; > + if (!(type&(ZEROPAD+LEFT))) > + while(size-->0) > + //*str++ = ' '; > + WRITE_CHAR_RET(' '); > + if (sign) > + //*str++ = sign; > + WRITE_CHAR_RET(sign); > + if (type & SPECIAL) { > + if (base==8) > + WRITE_CHAR_RET('0'); > + else if (base==16) { > + WRITE_CHAR_RET('0'); > + WRITE_CHAR_RET(digits[33]); > + } > + } > + if (!(type & LEFT)) > + while (size-- > 0) > + WRITE_CHAR_RET(c); > + while (i < precision--) > + WRITE_CHAR_RET('0'); > + while (i-- > 0) > + WRITE_CHAR_RET(tmp[i]); > + while (size-- > 0) > + WRITE_CHAR_RET(' '); > + return str; > +} > + > +static int mad_vsnprintf(char *s, size_t n, void *buf, const char *fmt, > va_list args) > +{ > + int rc = 0; > + int len; > + unsigned long long num; > + int i, base; > + char *str; > + > +/* Macros allows for bounding length of print to provided buffer > + * remove 1 to allow for \0 char */ > +#define WRITE_CHAR(c) do { \ > + *str++ = c; \ > + if (++rc >= (n-1)) { \ > + goto max_len_hit; \ > + } \ > +} while(0) > +#define WRITE_STR(STR) do { \ > + const char *ls = STR; \ > + len = strlen(ls); \ > + if (precision > 0 && len > precision) \ > + len = precision; \ > + if (!(flags & LEFT)) \ > + while (len < field_width--) \ > + WRITE_CHAR(' '); \ > + for (i = 0; i < len; ++i) \ > + WRITE_CHAR(*ls++); \ > + while (len < field_width--) \ > + WRITE_CHAR(' '); \ > +} while (0); > + > + int flags; > + int field_width; > + int precision; > + int qualifier; > + > + for (str=s ; *fmt ; ++fmt) { > + if (*fmt != '%') { > + //*str++ = *fmt; > + WRITE_CHAR(*fmt); > + continue; > + } > + > + /* process flags */ > + flags = 0; > +repeat: > + ++fmt; /* this also skips first '%' */ > + switch (*fmt) { > + case '-': flags |= LEFT; goto repeat; > + case '+': flags |= PLUS; goto repeat; > + case ' ': flags |= SPACE; goto repeat; > + case '#': flags |= SPECIAL; goto repeat; > + case '0': flags |= ZEROPAD; goto repeat; > + } > + > + /* get field width */ > + field_width = -1; > + if ('0' <= *fmt && *fmt <= '9') { > + int c = 0; > + for (field_width = 0; '0' <= (c = *fmt) && c <= '9'; > ++fmt) > + field_width = field_width*10 + c - '0'; > + } else if (*fmt == '*') { > + ++fmt; > + /* it's the next argument */ > + field_width = va_arg(args, int); > + if (field_width < 0) { > + field_width = -field_width; > + flags |= LEFT; > + } > + } > + > + /* get the precision */ > + precision = -1; > + if (*fmt == '.') { > + ++fmt; > + if ('0' <= *fmt && *fmt <= '9') { > + int c = 0; > + for (precision = 0; '0' <= (c = *fmt) && c <= > '9'; ++fmt) > + precision = precision*10 + c - '0'; > + } else if (*fmt == '*') { > + ++fmt; > + /* it's the next argument */ > + precision = va_arg(args, int); > + } > + if (precision < 0) > + precision = 0; > + } > + > + /* get the conversion qualifier */ > + qualifier = -1; > + if (*fmt == 'h' || *fmt == 'l' || *fmt == 'L' || *fmt =='z') { > + qualifier = *fmt; > + ++fmt; > + } > + > + /* default base */ > + base = 10; > + > + switch (*fmt) { > + case 'F': > + { > + char s[256]; > + uint8_t val[MAD_FIELD_MAX_BYTE_LEN]; > + int field = va_arg(args, int); > + const ib_field_t *f = ib_mad_f + field; > + > + if (field <= IB_NO_FIELD || field >= IB_FIELD_LAST_) > + continue; > + > + mad_decode_field(buf, field, val); > + f->def_dump_fn(s, n, val, ALIGN(f->bitlen, 8) / 8); > + WRITE_STR(s); > + continue; > + } > + case 'c': > + if (!(flags & LEFT)) > + while (--field_width > 0) > + WRITE_CHAR(' '); > + WRITE_CHAR((unsigned char) va_arg(args, int)); > + while (--field_width > 0) > + WRITE_CHAR(' '); > + continue; > + > + case 's': > + { > + const char *string = va_arg(args, char *); > + if (!string) > + string = "<NULL>"; > + > + WRITE_STR(string); > + continue; > + } > + > + case '%': > + WRITE_CHAR('%'); > + continue; > + > + /* integer number formats - set up the flags and "break" */ > + case 'o': > + base = 8; > + break; > + > + case 'p': > + case 'X': > + flags |= LARGE; > + case 'x': > + base = 16; > + break; > + > + case 'd': > + case 'i': > + flags |= SIGN; > + case 'u': > + break; > + > + default: > + WRITE_CHAR('%'); > + if (*fmt) > + WRITE_CHAR(*fmt); > + else > + --fmt; > + continue; > + } > + if (qualifier == 'l') { > + num = va_arg(args, unsigned long); > + if (flags & SIGN) > + num = (signed long) num; > + } else if (qualifier == 'z') { > + num = va_arg(args, size_t); > + } else if (qualifier == 'h') { > + num = (unsigned short) va_arg(args, int); > + if (flags & SIGN) > + num = (signed short) num; > + } else { > + num = va_arg(args, unsigned int); > + if (flags & SIGN) > + num = (signed int) num; > + } > + str = number(str, n-rc, &rc, num, base, field_width, precision, > flags); > + if (rc <= 1) > + break; > + } > +max_len_hit: > + *str = '\0'; > + return str-s; > +} Now instead of reimplementing *printf() functions with potential need to follow their extensions/conventions/update/etc wouldn't it be easier (and in long term safer) to just rebuild format string by resolving known %X conversions and then to pass it with rest parameters to standard libc's *printf()? In this way we will support all what *printf()s know + our conversions. Sasha > + > +int mad_snprintf(char *s, size_t n, uint8_t *buf, const char *format, ...) > +{ > + va_list args; > + int i; > + > + va_start(args, format); > + i = mad_vsnprintf(s, n, buf, format, args); > + va_end(args); > + return (i); > +} > + > +int mad_fprintf(FILE *stream, uint8_t *buf, const char *format, ...) > +{ > + char str_buf[1024]; > + va_list args; > + int i,j; > + > + va_start(args, format); > + i = mad_vsnprintf(str_buf, 1024, buf, format, args); > + va_end(args); > + j = fprintf(stream, "%s", str_buf); > + if (i != j) > + IBWARN("mad_vsnprintf and fprintf don't match???\n"); > + return (i); > +} > + > diff --git a/libibmad/src/libibmad.map b/libibmad/src/libibmad.map > index 0be7a92..2265b12 100644 > --- a/libibmad/src/libibmad.map > +++ b/libibmad/src/libibmad.map > @@ -4,6 +4,8 @@ IBMAD_1.3 { > mad_dump_field; > mad_dump_val; > mad_print_field; > + mad_snprintf; > + mad_fprintf; > mad_dump_array; > mad_dump_bitfield; > mad_dump_hex; > -- > 1.5.4.5 > _______________________________________________ general mailing list [email protected] http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
