On Fri, 22 Feb 2013 20:00:59 +0000
"Hefty, Sean" <[email protected]> wrote:
> > > > +char * umad_class_str(uint8_t mgmt_class);
> > > > +char * umad_method_str(uint8_t mgmt_class, uint8_t method);
> > > > +char * umad_attribute_str(uint8_t mgmt_class, be16_t attr_id);
> > > > +char * umad_mad_status_str(be16_t status, char * buf, size_t len);
> > >
> > > Why is umad_mad_status_str() different?
> >
> > Which difference are you speaking of, the lack of mgmt_class or the required
> > buffer?
>
> I was referring to the buf/len input fields. Why are those needed? I didn't
> follow this reasoning:
>
> > The required buffer allows for printing multiple strings for various fields
> > which may be set. This is done because status is not defined to be a strict
> > enum.
>
> +char * umad_mad_status_str(be16_t status, char * buf, size_t len)
> +{
> + size_t n = len;
> + int rc = 0;
> +
> + status = ntohs(status);
> +
> + if (status & UMAD_STATUS_BUSY) {
> + rc = snprintf(buf, n, "busy");
> + if (rc < 0 || rc >= n)
> + goto done;
> + n -= rc;
> + buf += rc;
> + }
> +
> + if (status & UMAD_STATUS_REDIRECT) {
> + rc = snprintf(buf, n, " redirection required");
> + if (rc < 0 || rc >= n)
> + goto done;
> + n -= rc;
> + buf += rc;
> + }
> +
> + switch((status & 0x001C)) {
> + case UMAD_STATUS_BAD_VERSION:
> + rc = snprintf(buf, n, " bad version");
> + break;
> + case UMAD_STATUS_METHOD_NOT_SUPPORTED:
> + rc = snprintf(buf, n, " method not supported");
> + break;
> + case UMAD_STATUS_ATTR_NOT_SUPPORTED:
> + rc = snprintf(buf, n, " method/attribute combo not supported");
> + break;
> + case UMAD_STATUS_INVALID_ATTR_VALUE:
> + rc = snprintf(buf, n, " invalid attribute/modifier value");
> + break;
> + }
> +
> + if (rc < 0 || rc >= n)
> + goto done;
> + n -= rc;
> + buf += rc;
> +
> + if (n == len)
> + snprintf(buf, n, "Success");
> +done:
> + return (buf);
> +}
>
> These return values seem mutually exclusive. Surely we're not going to see a
> MAD with status 'busy - redirection required - bad version - success'?
They are mutually exclusive now but because the spec defines separate fields it
might not always be that way. That is what I meant when I said "status is not
defined to be a strict enum".
Ira
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Ira Weiny
Member of Technical Staff
Lawrence Livermore National Lab
925-423-8008
[email protected]
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
