Messages in this thread | | | Date | Sat, 28 Apr 2007 09:58:57 +0100 | From | Simon Arlott <> | Subject | Re: [PATCH (rev 2)] cxacru: Cleanup sysfs attribute code |
| |
On 28/04/07 08:50, Andrew Morton wrote: > On Wed, 25 Apr 2007 19:41:20 +0100 Simon Arlott <simon@fire.lp0.eu> wrote: >> This changes the format of unknown status values to be less verbose and >> uses an array instead of several different snprintf calls. Since only >> enum values are assigned to it, poll_state is changed from int to enum. >> Use abs() for dB values instead of two almost identical return lines. >> ... >> + static char *str[] = { "no", "yes" }; >> + if (unlikely(value >= ARRAY_SIZE(str))) >> + return snprintf(buf, PAGE_SIZE, "%u\n", value); >> + return snprintf(buf, PAGE_SIZE, "%s\n", str[value]); > > Should a bool be displayed as "true" or "false"?
The only attribute that uses this is line_startable... I suppose it could return true/false - I just happened to use yes/no.
>> + return snprintf(buf, PAGE_SIZE, "%u\n", value); > > To be completely pedantic: we shouldn't be printing u32's with %u. Because > %u assumes that u32 is implemented as unsigned int. Only it's an opaque > type and we don't know what actual C type the architecture chose to use. > > It happens to work OK on all architectures and I expect it always will, so > no change is needed, but there you have it.
Ok.
> u64's and %llu _are_ incompatible on some architectures and I get to fix > that about 1000000 times.
-- Simon Arlott - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |