Re: C internals cleanup

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: C internals cleanup

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:46:35

Andy Lester [off-list ref] writes:
I've been poking around in the source for git, and wanted to pitch in
and clean some things up.

Two biggies that tend to get overlooked: Applying the const keyword
where possible, and localizing variables to innermost blocks.

Also, want to to get a target going in the Makefile for running under
splint.

Just want to make sure my internal cleanups are not going to be seen
as a nuisance.
I and my lieutenants are most likely polite enough to avoid using a word
like nuisance, but I think you will hear words like code churn, especially
if such a change affects too many places and conflicts with too many
patches that are still in the queue to be merged.  IOW, you need to be
careful.

The general rule of thumb is to do such a clean-up before you start to
work on something of substance.

Your series may look like this:

 * [Patch 1/2] hello.c: tighten constness and scope

   Many functions in this file do not have to be called from other files,
   and goodmorning() takes "char *" parameters but does not modify them in
   any way (others like goodafternoon() and goodevening() are already
   correct in this regard, both taking "const char *" parameters).

   Make all of tese functions file scope static, and tighten constness to
   the parameters to gootmorning().  By making their function signatures
   compatible, this change will help the next patch that refactors the
   three greeting functions.

 * [Patch 2/2] hello.c: refactor goodmorning(), goodafternoon() and goodnight()

   These functions do mostly the same thing; implement a common helper
   function greeting() and share code among them.

Otherwise, unless the tree is really quiet, a patch that is only clean-up
and nothing else will have a high maintenance-cost vs reward ratio, and
needs to be split and timed carefully.  A patch that applies cleanly to
master and then the result merges cleanly to both next and pu would be Ok
even if you do not add any value other than code cleanliness.

Re: C internals cleanup

From: Andy Lester <hidden>
Date: 2016-06-15 22:46:35

The general rule of thumb is to do such a clean-up before you start to
work on something of substance.
I guess that's all in how one defines "substance." :-)

My ultimate goal is to get more stringent error-checking, to get more  
compiler warnings enabled, and to get a splint target.  splint is  
fantastic for tracking all sorts of memory and logic problems, but  
benefits greatly from getting the const qualifiers in place.  Even gcc  
will be happier.

Thanks for the recap.  It's hard to absorb what you just described  
just from reading mailing list history.

xoa

--
Andy Lester => andy@petdance.com => www.petdance.com => AIM:petdance
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help