Re: [PATCH 2/7] proc: Modify proc_pident_lookup to be completely table driven.

[Eric W. Biederman]

Dave Hansen <haveblue@xxxxxxxxxx> writes:

> On Tue, 2006-08-15 at 12:05 -0600, Eric W. Biederman wrote:
>> Currently proc_pident_lookup gets the names and types from a table
>> and then has a huge switch statement to get the inode and file
>> operations it needs.  That is silly and is becoming increasingly hard
>> to maintain so I just put all of the information in the table.
>
> Looks pretty reasonable.
>
>> +#define INF(TYPE, NAME, MODE, OTYPE)                   \
>> +       NOD(TYPE, NAME, (S_IFREG|(MODE)),               \
>> +               NULL, &proc_info_file_operations,       \
>> +               { .proc_read = &proc_##OTYPE } )
> ...
>> +       INF(PROC_TID_OOM_SCORE,  "oom_score", S_IRUGO, oom_score),
>> +       REG(PROC_TID_OOM_ADJUST, "oom_adj",   S_IRUGO|S_IWUSR, oom_adjust),
>
> Could we give these some slightly more intuitive names?  INF is a bit
> terse ;)

> Since these #defines and function are also all in base.c, and not
> referenced elsewhere, might it be reasonable to take some of the PROC_
> headers off of them?  I know I've been frustrated more than once by
> popping things like "oom_score" in to cscope and finding no definitions.

So as for taking the proc_ prefix off I don't see a real problem with
that but that should really be a different increment patch.  Although removing
a little magic from my macros might be reasonable.

The only macro whose name really seems to terse is INF short for info,
so adding the O would be ok.  What I would really like to see however
is all of these proc info things just go away.  I seem to recall
they all have some pretty weird corner cases and a seq file implementation
is probably better there.

The other half of this is that I would very much like to see some of
these things moved out of base.c

Eric
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/