Messages in this thread | | | Date | Thu, 10 Apr 2008 08:57:40 -0500 | From | Paul Jackson <> | Subject | Re: [PATCH 1/3] x86: modify show_shared_cpu_map in intel_cacheinfo v3 |
| |
Mike wrote: + n = type? + cpulist_scnprintf(buf, len-2, *mask): + cpumask_scnprintf(buf, len-2, *mask);
I suspect most of us would find the following variant easier to read:
if (type) n = cpulist_scnprintf(buf, len - 2, *mask); else n = cpumask_scnprintf(buf, len - 2, *mask);
Then, going further, the rather too vague "type" parameter name, without comment and taking just bare constant values 0 or 1, seems more opaque than necessary.
I can imagine this being easier to read as something like:
typedef enum { print_as_mask, print_as_list } map_printer_t;
static ssize_t show_shared_cpu_map_func(struct _cpuid4_info *this_leaf, map_printer_t mpt, char *buf) { ptrdiff_t len = PTR_ALIGN(buf + PAGE_SIZE - 1, PAGE_SIZE) - buf; int n = 0;
if (len > 1) { cpumask_t *mask = &this_leaf->shared_cpu_map;
if (mpt == print_as_mask) n = cpumask_scnprintf(buf, len - 2, *mask); else n = cpulist_scnprintf(buf, len - 2, *mask); buf[n++] = '\n'; buf[n] = '\0'; } return n; }
static inline ssize_t show_shared_cpu_map(struct _cpuid4_info *leaf, char *buf) { return show_shared_cpu_map_func(leaf, print_as_mask, buf); }
static inline ssize_t show_shared_cpu_list(struct _cpuid4_info *leaf, char *buf) { return show_shared_cpu_map_func(leaf, print_as_list, buf); }
-- I won't rest till it's the best ... Programmer, Linux Scalability Paul Jackson <pj@sgi.com> 1.940.382.4214
| |