On Mon, May 02, 2011 at 08:46:59PM +0200, Sami Kerola wrote:
> - for (cnt = 0, lp = list; cnt < entries; ++cnt, ++lp, ++t) {
> - for (coloff = 0, p = *lp;
> - (cols[coloff] = wcstok(p, separator, &wcstok_state)) != NULL;
> - p = NULL) {
> + lp = list;
> +
> + for (cnt = 0; cnt < entries; ++cnt, ++lp, ++t) {
Is it really more readable to have 'lp = list' outside the for() ?
> + coloff = 0;
> + p = *lp;
> + while ((cols[coloff] = wcstok(p, separator, &wcstok_state)) != NULL) {
> if (++coloff == maxcols) {
> - cols = xrealloc(cols, ((u_int)maxcols + DEFCOLS)
> - * sizeof(wchar_t *));
> - lens = xrealloc(lens, ((u_int)maxcols + DEFCOLS)
> - * sizeof(int));
> - memset((char *)lens + maxcols * sizeof(int),
> - 0, DEFCOLS * sizeof(int));
> maxcols += DEFCOLS;
> + cols = xrealloc(cols, maxcols * sizeof(wchar_t *));
> + lens = xrealloc(lens, maxcols * sizeof(int));
> + /* zero fill only new memory */
> + memset(lens + (maxcols - DEFCOLS), 0,
This seems incorrect. The memset() works with bytes, but you forgot
sizeof(int), I think it should be:
lens + ((maxcols - DEFCOLS) * sizeof(int))
Wouldn't be possible to use ssize_t also for 'lens'?
BTW, what about to add xrealloc0() which zeroize the new allocated
memory? It seems better than have memset() on many places where we
have xrealloc().
> + DEFCOLS * sizeof(int));
> - for (cnt = 0, t = tbl; cnt < entries; ++cnt, ++t) {
> +
> + t = tbl;
> +
> + for (cnt = 0; cnt < entries; ++cnt, ++t) {
The same thing, the old code was nice from my point of view :-))
> for (coloff = 0; coloff < t->cols - 1; ++coloff) {
> fputws(t->list[coloff], stdout);
> for (i = lens[coloff] - t->len[coloff] + 2; i > 0; i--)
Note, you don't have to resend all series of the patches (just this
one only), I'll pull from your git repository. Thanks!
Karel
--
Karel Zak <kzak@xxxxxxxxxx>
http://karelzak.blogspot.com
--
To unsubscribe from this list: send the line "unsubscribe util-linux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
