On Thu, 2011-08-25 at 13:00 +0530, Vasanthakumar Thiagarajan wrote:
> Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@xxxxxxxxxxxxxxxx>
[]
> diff --git a/drivers/net/wireless/ath/ath6kl/debug.c b/drivers/net/wireless/ath/ath6kl/debug.c
[]
> +static ssize_t read_file_credit_dist_stats(struct file *file,
> + char __user *user_buf,
> + size_t count, loff_t *ppos)
> +{
> + struct ath6kl *ar = file->private_data;
> + struct htc_target *target = ar->htc_target;
> + struct htc_endpoint_credit_dist *ep_list;
> + char *buf;
> + unsigned int buf_len = 128, len = 0;
> + ssize_t ret_cnt;
> +
> + buf_len += get_queue_depth(&target->cred_dist_list) * 128;
Doesn't look like 128 is the right size.
> + buf = kzalloc(buf_len, GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
kmalloc can run out of space pretty quickly if cred_dist_list
is large. Maybe vmalloc?
> +
> + len += scnprintf(buf + len, buf_len - len,
> + " Epid Flags Cred_norm Cred_min Credits Cred_assngd"
> + " Seek_cred Cred_sz Cred_per_msg Cred_to_dist"
> + " qdepth\n");
> +
> + list_for_each_entry(ep_list, &target->cred_dist_list, list) {
> + len += scnprintf(buf + len, buf_len - len, " %2d",
> + ep_list->endpoint);
> + len += scnprintf(buf + len, buf_len - len, "%10x",
> + ep_list->dist_flags);
I think this is nearly unreadable.
It doesn't line up well with your header.
Why not align them correctly or otherwise make no
attempt to align them at all?
Your current header and field widths:
1 2 3 4 5 6 7 8 9 10 11 12
123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890
len += scnprintf(buf + len, buf_len - len,
" Epid Flags Cred_norm Cred_min Credits Cred_assngd Seek_cred Cred_sz Cred_per_msg Cred_to_dist qdepth\n");
12 12345678 123456789 1234567890123 123456789 123456789012
1234567890 123456789 1234567890 123456789012 12345678901234
> + list_for_each_entry(ep_list, &target->cred_dist_list, list) {
> + len += scnprintf(buf + len, buf_len - len, " %2d",
> + ep_list->endpoint);
> + len += scnprintf(buf + len, buf_len - len, "%10x",
> + ep_list->dist_flags);
maybe use a macro:
#define ep_list_field(fmt, field) \
len += scnprint(buf + len, buf_len - len, fmt, ep_list->field)
ep_list_field(" %2d", endpoint);
ep_list_field("%9d", cred_min);
ep_list_field("%9d", credits);
etc
> + len += scnprintf(buf + len, buf_len - len, "%12d\n",
> + get_queue_depth(&((struct htc_endpoint *)
> + ep_list->htc_rsvd)->txq));
> + }
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
