threads / discuss / 18844

C internals cleanup

Subject: C internals cleanup

## tl;dr

3 messages between Apr 13, 2009 and Apr 13, 2009.

replies: 2people: 2as markdown or json

Andy Lester· Apr 13, 2009, 03:15 UTC · lore

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.

xoxo, Andy

-- Andy Lester => andy@petdance.com => www.petdance.com => AIM:petdance

Junio C Hamano· Apr 13, 2009, 04:06 UTC · re: Andy Lester · lore

Re: C internals cleanup

Andy Lester <andy@petdance.com> writes:
Show 11 quoted lines
> 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.

Andy Lester· Apr 13, 2009, 04:29 UTC · re: Junio C Hamano · lore

Re: C internals cleanup

> 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

← back to recent threads