[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] fix new format when printing multiple lines
From: |
Jim Meyering |
Subject: |
Re: [PATCH] fix new format when printing multiple lines |
Date: |
Sun, 22 Dec 2013 18:07:16 -0800 |
Hi Sami,
Thanks for working on this.
After you pushed that, I noticed a couple things that
I'd do differently. Since the new print_box function
simply prints the specified message, that message
parameter should be a "const" pointer. The only reason
we can't simply add the "const" is because the current
implementation happens to modify it, incidentally.
It uses wcstok to extract the lines, and that function
writes into the string being parsed. It is better both
for efficiency and aesthetics to avoid use of that function,
just as it's good to avoid use of strtok. Instead, I would
suggest to use wcschr. Then the parameter can be the
more expected "const" pointer. Also, with sizeof, it is
more maintainable to use xmalloc(sizeof *VAR) than
xmalloc(sizeof TYPE), since that avoids the risk of
error, e.g., when someone changes the type of VAR from
TYPE to NEW_TYPE and forgets to update this use of the
type name.
I've attached a suggested starting point (doesn't even
compile warning-free yet -- needs the wcschr change
I mentioned), in case you feel like working on this.
Thanks,
Jim
k.txt
Description: Text document
- [PATCH] fix new format when printing multiple lines, Sami Kerola, 2013/12/04
- Re: [PATCH] fix new format when printing multiple lines, Reuben Thomas, 2013/12/04
- Re: [PATCH] fix new format when printing multiple lines, Sami Kerola, 2013/12/04
- Re: [PATCH] fix new format when printing multiple lines, Sami Kerola, 2013/12/10
- Re: [PATCH] fix new format when printing multiple lines,
Jim Meyering <=
- Re: [PATCH] fix new format when printing multiple lines, Karl Berry, 2013/12/23
- Re: [PATCH] fix new format when printing multiple lines, Reuben Thomas, 2013/12/23
- Re: [PATCH] fix new format when printing multiple lines, Reuben Thomas, 2013/12/23
- Re: [PATCH] fix new format when printing multiple lines, Karl Berry, 2013/12/23
- Re: [PATCH] fix new format when printing multiple lines, Karl Berry, 2013/12/23
- Re: [PATCH] fix new format when printing multiple lines, Reuben Thomas, 2013/12/23
- Re: [PATCH] fix new format when printing multiple lines, Sami Kerola, 2013/12/25
- Re: [PATCH] fix new format when printing multiple lines, Karl Berry, 2013/12/25
- Re: [PATCH] fix new format when printing multiple lines, Reuben Thomas, 2013/12/25
- Re: [PATCH] fix new format when printing multiple lines, Sami Kerola, 2013/12/25