lkml.org 
[lkml]   [2008]   [Apr]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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


    \
     
     \ /
      Last update: 2008-04-10 16:01    [W:0.021 / U:0.328 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site