{"thread":{"id":"18844","subject":"C internals cleanup","startedAt":"2009-04-13T03:15:05Z","lastAt":"2009-04-13T04:29:16Z","messageCount":3,"participants":["Andy Lester","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"111155","messageId":"22578EEA-DB8B-4DAF-B217-FF13DC8A3EC7@petdance.com","threadId":"18844","inReplyTo":null,"subject":"C internals cleanup","fromName":"Andy Lester","fromEmail":"andy@petdance.com","sentAt":"2009-04-13T03:15:05Z","receivedAt":"2009-04-13T03:15:05Z","isPatch":false,"sender":{"key":"andy@petdance.com","avatar":"https://gravatar.com/avatar/997ccaea115635c2be5051f455bbe6287c0f13aa59c96e1d6eb21ba3589239fb?d=mp&s=160"},"body":"I've been poking around in the source for git, and wanted to pitch in  \nand clean some things up.\n\nTwo biggies that tend to get overlooked: Applying the const keyword  \nwhere possible, and localizing variables to innermost blocks.\n\nAlso, want to to get a target going in the Makefile for running under  \nsplint.\n\nJust want to make sure my internal cleanups are not going to be seen  \nas a nuisance.\n\nxoxo,\nAndy\n\n--\nAndy Lester => andy@petdance.com => www.petdance.com => AIM:petdance\n"},{"id":"111160","messageId":"7v4owtw623.fsf@gitster.siamese.dyndns.org","threadId":"18844","inReplyTo":"22578EEA-DB8B-4DAF-B217-FF13DC8A3EC7@petdance.com","subject":"Re: C internals cleanup","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-13T04:06:44Z","receivedAt":"2009-04-13T04:06:44Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andy Lester <andy@petdance.com> writes:\n\n> I've been poking around in the source for git, and wanted to pitch in\n> and clean some things up.\n>\n> Two biggies that tend to get overlooked: Applying the const keyword\n> where possible, and localizing variables to innermost blocks.\n>\n> Also, want to to get a target going in the Makefile for running under\n> splint.\n>\n> Just want to make sure my internal cleanups are not going to be seen\n> as a nuisance.\n\nI and my lieutenants are most likely polite enough to avoid using a word\nlike nuisance, but I think you will hear words like code churn, especially\nif such a change affects too many places and conflicts with too many\npatches that are still in the queue to be merged.  IOW, you need to be\ncareful.\n\nThe general rule of thumb is to do such a clean-up before you start to\nwork on something of substance.\n\nYour series may look like this:\n\n * [Patch 1/2] hello.c: tighten constness and scope\n\n   Many functions in this file do not have to be called from other files,\n   and goodmorning() takes \"char *\" parameters but does not modify them in\n   any way (others like goodafternoon() and goodevening() are already\n   correct in this regard, both taking \"const char *\" parameters).\n\n   Make all of tese functions file scope static, and tighten constness to\n   the parameters to gootmorning().  By making their function signatures\n   compatible, this change will help the next patch that refactors the\n   three greeting functions.\n\n * [Patch 2/2] hello.c: refactor goodmorning(), goodafternoon() and goodnight()\n\n   These functions do mostly the same thing; implement a common helper\n   function greeting() and share code among them.\n\nOtherwise, unless the tree is really quiet, a patch that is only clean-up\nand nothing else will have a high maintenance-cost vs reward ratio, and\nneeds to be split and timed carefully.  A patch that applies cleanly to\nmaster and then the result merges cleanly to both next and pu would be Ok\neven if you do not add any value other than code cleanliness.\n"},{"id":"111161","messageId":"A60582E3-0374-4922-B6C9-42BCF2DCAFFB@petdance.com","threadId":"18844","inReplyTo":"7v4owtw623.fsf@gitster.siamese.dyndns.org","subject":"Re: C internals cleanup","fromName":"Andy Lester","fromEmail":"andy@petdance.com","sentAt":"2009-04-13T04:29:16Z","receivedAt":"2009-04-13T04:29:16Z","isPatch":false,"sender":{"key":"andy@petdance.com","avatar":"https://gravatar.com/avatar/997ccaea115635c2be5051f455bbe6287c0f13aa59c96e1d6eb21ba3589239fb?d=mp&s=160"},"body":"> The general rule of thumb is to do such a clean-up before you start to\n> work on something of substance.\n\nI guess that's all in how one defines \"substance.\" :-)\n\nMy ultimate goal is to get more stringent error-checking, to get more  \ncompiler warnings enabled, and to get a splint target.  splint is  \nfantastic for tracking all sorts of memory and logic problems, but  \nbenefits greatly from getting the const qualifiers in place.  Even gcc  \nwill be happier.\n\nThanks for the recap.  It's hard to absorb what you just described  \njust from reading mailing list history.\n\nxoa\n\n--\nAndy Lester => andy@petdance.com => www.petdance.com => AIM:petdance\n"}]}