Hi,
while reading Documentation/CodingStyle for the nth time, I realized that I had
read some conflicting coding style in some patch posted to the linux-kernel
mailing-list; in include/linux/page-flags.h, there is a lot of defines that are
apparently frowned upon:
HOWEVER, while mixed-case names are frowned upon, descriptive names for
global variables are a must. To call a global function "foo" is a
shooting offense.
Are those an exception to the rule or would for example PF_LOCKED/pf_locked be
a nice replacement for PageLocked?
Just wondering; with no intention to change code that I do not understand :)
Kenneth
On Wed, Apr 06, 2005 at 03:29:21AM +0200, Kenneth Aafl?y wrote:
> Hi,
>
> while reading Documentation/CodingStyle for the nth time, I realized that I had
> read some conflicting coding style in some patch posted to the linux-kernel
> mailing-list; in include/linux/page-flags.h, there is a lot of defines that are
> apparently frowned upon:
>
> HOWEVER, while mixed-case names are frowned upon, descriptive names for
> global variables are a must. To call a global function "foo" is a
> shooting offense.
>
> Are those an exception to the rule or would for example
> PF_LOCKED/pf_locked be a nice replacement for PageLocked?
While there may be reasons why mixed case is suboptimal, the real
reason is that it's hard to keep track of which style is used where.
It's annoying and error-prone to have to remember the naming format
for everything in addition to its name. As most things are in a
standard style, things are made easier by having every piece of new
code follow that style and let us slowly approach uniformity.
If you posted a patch for pf_locked() and friends (and note that it's
lowercase to match function-like usage), you'd probably find some
enthusiasts and some naysayers. Most of the naysayers would object on
the grounds of "it ain't broke", but if someone were to do it as part
of a series of more substantial clean-ups, it'd likely be accepted.
--
Mathematics is the supreme nostalgia of our time.
On Wednesday 06 April 2005 04:09, Matt Mackall wrote:
> While there may be reasons why mixed case is suboptimal, the real
> reason is that it's hard to keep track of which style is used where.
> It's annoying and error-prone to have to remember the naming format
> for everything in addition to its name. As most things are in a
> standard style, things are made easier by having every piece of new
> code follow that style and let us slowly approach uniformity.
My primary concern was that of; why does the kernels own coding style
deviate from that advise given in it's documentation. Other than that
most mixed-case errors will be caught by the compiler, unless there
is an ambiguity with other mixed-case statements; which is probably
why that clause exists in the coding style documentation.
> If you posted a patch for pf_locked() and friends (and note that it's
> lowercase to match function-like usage), you'd probably find some
> enthusiasts and some naysayers. Most of the naysayers would object on
> the grounds of "it ain't broke", but if someone were to do it as part
> of a series of more substantial clean-ups, it'd likely be accepted.
Certainly I would like to have a go at a patch, but I must say that I do not
feel particularly familiar with the code in question to make such a change.
I would have risen to the challenge had this been a driver level change,
but the mmu is something that I will not touch untill I feel comfortable.
I feel that this is a change that would be best managed by an experienced
kernel janitor.
Kenneth
Kenneth Aafl?y wrote:
> On Wednesday 06 April 2005 04:09, Matt Mackall wrote:
>
>>While there may be reasons why mixed case is suboptimal, the real
>>reason is that it's hard to keep track of which style is used where.
>>It's annoying and error-prone to have to remember the naming format
>>for everything in addition to its name. As most things are in a
>>standard style, things are made easier by having every piece of new
>>code follow that style and let us slowly approach uniformity.
>
>
> My primary concern was that of; why does the kernels own coding style
> deviate from that advise given in it's documentation. Other than that
Probably it's been like that for a long time, and nobody has
really bothered to change it.
>>If you posted a patch for pf_locked() and friends (and note that it's
>>lowercase to match function-like usage), you'd probably find some
>>enthusiasts and some naysayers. Most of the naysayers would object on
>>the grounds of "it ain't broke", but if someone were to do it as part
>>of a series of more substantial clean-ups, it'd likely be accepted.
>
>
> Certainly I would like to have a go at a patch, but I must say that I do not
> feel particularly familiar with the code in question to make such a change.
> I would have risen to the challenge had this been a driver level change,
> but the mmu is something that I will not touch untill I feel comfortable.
Well the only patch that could possibly be considered would be a
straight search and replace, and absolutely no functional changes;
I think you would be up to it ;)
A few suggestions:
Don't use PF_*. That namespace is already being used by at least
process flags and protocol flags. Maybe page_locked, page_dirty,
etc. might be better
There could be a quite a bit of external code using these interfaces.
Typically we wouldn't just rename public interfaces in a stable
series "just because", but the rules are a bit different for 2.6.
Your best bet would be to firstly do a patch to create the new interface
names but keep the old ones in place for backwards compatibility (just
#defined to the new name), then a second patch to convert over all the
in-kernel users. The compatibility stuff can be removed in N years.
Lastly, it is quite likely that many people will consider this to be
more trouble than it's worth. So keep in mind it is not guaranteed to
get included.
--
SUSE Labs, Novell Inc.
On Wed, 6 Apr 2005, Nick Piggin wrote:
> Don't use PF_*. That namespace is already being used by at least
> process flags and protocol flags. Maybe page_locked, page_dirty,
> etc. might be better
Much better. But...
> Lastly, it is quite likely that many people will consider this to be
> more trouble than it's worth. So keep in mind it is not guaranteed to
> get included.
Count me among them - IMHO it's a worthless change.
Hugh