[ofa-general] ***SPAM*** Re: [PATCH] Update mad formatting functions.
Sasha Khapyorsky
sashak at voltaire.com
Wed Apr 15 08:30:03 PDT 2009
Hi Ira,
On 14:44 Wed 11 Mar , Ira Weiny wrote:
>
> From: Ira Weiny <weiny2 at llnl.gov>
> 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 <weiny2 at llnl.gov>
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 <weiny2 at llnl.gov>
> 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
>
More information about the general
mailing list