On 08/01/16 10:24, Piotr Stefaniak wrote:
On 2016-08-01 08:08, Bruce Evans wrote:
On Sun, 31 Jul 2016, Xin Li wrote:
On 7/31/16 14:36, Pedro F. Giffuni wrote:
- bzero(f, sizeof *f);
+ memset(f, 0, sizeof(struct fstate));
^^^^^^^^^^^^^^^^^^^^^ This is much more error-prone
than sizeof(*f) IMHO.
I also prefer bzero().
I hope this is merely a preference and not a hard rule, because I'm of
the opinion that the memset()-based equivalent of bzero() has fewer
portability consequences, which is worth paying attention to. Please
consider the fact that NetBSD has done this replacement.
It is a preference, not a hard rule.
In general, replacing bzero with memset is seen as a portability
enhancement but bzero is not going away from FreeBSD and replacing it
is not a priority.
I do agree that replacing the expression with the type name was a
regression; it was my mistake.
Removal of the space after sizeof is another regression. KNF disallows
the space, but indent's style is very far from KNF. It isn't clear if
indent's style is to require the space, since old versions of indent
never used sizeof(typename), and 'sizeof object' requires the space.
I was specifically asked in the D6966 differential review to adhere to
style(9). I have changed both my own code submitted for review and the
rest of style violations of this kind as a separate patch
(https://github.com/pstef/freebsd_indent/commit/a2befd74fa54c91d96a38317e90d38ef17682f4b).
I had expected the style fixes to get committed before the change in
r303600, in belief that doing so would render possible complaints as the
one quoted above as not relevant anymore.
The idea is that new code should try to adhere to style(9) but it is
not an obligation to do so if the codebase already uses a different
style.
Regressions started in r93440 with sizeof(object) in an nitems() expansion.
The style of this is very different from an nitems() expansion in r1590.
There was 1 more sizeof(object) and 1 sizeof(int). This is the first
sizeof(typename) where 'sizeof object' cannot be used for technical
reasons.
KNF also requires parentheses for sizeof(object). Then the space is
unnecessary and disallowed.
On a more general note, I imagined we're heading towards slowly changing
indent(1)'s code to make it more style(9)-compliant (not least because
it's tempting to imagine indent(1) being able to re-indent itself in
accordance with style(9) at some point) but right now I'm confused as to
what style decisions in the patches I submit are expected of me.
You are doing a great job!
In the case of indent, while it would be desirable to move it all to
KNF, that involves way too many cosmetic changes that have no use.
Reviewers can get things wrong, and in general it is up to the
committer, IMO, style cleanups are generally very low priority.
It is preferable to have smaller to-the-point changes than style
issues mixed with effective code changes.
Going out of your way to clean indent style --> KNF is a waste of time.
IMHO, of course.
Pedro.
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
