LinuxLists.cc - [RFC][PATCH] unchecked request

[permalink] [raw]

Subject: Re: [RFC][PATCH] unchecked request_region's in drivers/net

Em Thu, Dec 27, 2001 at 02:38:12PM -0600, Steven Walter escreveu:
> This patch corrects the request_region calls in drivers/net to have
> their return value checked, and fail appropriately. Additionally, the
> check_region calls have been removed where prudent.
>
> I'd like volunteers to test and comment on the patch; if the general consensus is that it's good, I'd like to see it integrated.
>
> Patch is against kernel 2.4.17, should apply to 2.5 as well.

Good job! But please consider splitting the patch per driver and sending it
to the respective maintainers.

- Arnaldo

2001-12-27 22:44:31

[permalink] [raw]

Subject: Re: [RFC][PATCH] unchecked request_region's in drivers/net

On Thu, 27 Dec 2001, Arnaldo Carvalho de Melo wrote:

> > Patch is against kernel 2.4.17, should apply to 2.5 as well.
> Good job! But please consider splitting the patch per driver and sending it
> to the respective maintainers.

Someone with far too much time on their hands would be my personal
hero[*] if they were to write a script (in language of their choice) to
parse a diff, extract filename, and do lookup in a flat text file
to find a list of maintainers/interested parties.

Imagine a patch against devfs..

$ cclist my.devfs.patch.diff
Richard Gooch <[email protected]>
Alexander Viro <[email protected]>

SCNR 8-)

This 'little black book of addresses' doesn't have to be anything
wonderful, but its tedious work for someone to make the textfile
mapping the various source files to email addresses.

Someone (Alan?) suggested having something like a web interface
allowing anyone interested in any particular file to register
their interest, and get added to the cclist for that file.
Which is also a cool idea.

Dave.

[*] At least for a while. I'm fickle.

--
| Dave Jones. http://www.codemonkey.org.uk
| SuSE Labs

2001-12-27 22:51:42

[permalink] [raw]

Subject: Re: [RFC][PATCH] unchecked request_region's in drivers/net

Em Thu, Dec 27, 2001 at 11:44:12PM +0100, Dave Jones escreveu:
> On Thu, 27 Dec 2001, Arnaldo Carvalho de Melo wrote:
>
> > > Patch is against kernel 2.4.17, should apply to 2.5 as well.
> > Good job! But please consider splitting the patch per driver and sending it
> > to the respective maintainers.
>
> Someone with far too much time on their hands would be my personal
> hero[*] if they were to write a script (in language of their choice) to
> parse a diff, extract filename, and do lookup in a flat text file
> to find a list of maintainers/interested parties.

Humm, wouldn't it be the case to update MAINTAINERS? Or having something
like another file INTERESTED or other better name with addresses of the
right person/mailing list to send patches to?

But I like the idea of a web interface so that people interested in
specific drivers/files could register and receive it when patches are
submitted, maybe this can be coupled with the patchbot idea, that would
lookup this database (plain text file, don't worry) and send the message to
the interested people.

/me thinks... I think we have this in our linux distribution buildsystem,
for people to register interest in particular packages, but that may well
be coupled with bugzilla, and that would be too much for this simple
system... ok, I'll look into this with the distro folks here.

- Arnaldo

2001-12-27 23:25:27

by James A Sutherland

[permalink] [raw]

Subject: Re: [RFC][PATCH] unchecked request_region's in drivers/net

On Thursday 27 December 2001 10:44 pm, Dave Jones wrote:
> On Thu, 27 Dec 2001, Arnaldo Carvalho de Melo wrote:
> > > Patch is against kernel 2.4.17, should apply to 2.5 as well.
> >
> > Good job! But please consider splitting the patch per driver and sending
> > it to the respective maintainers.
>
> Someone with far too much time on their hands would be my personal
> hero[*] if they were to write a script (in language of their choice) to
> parse a diff, extract filename, and do lookup in a flat text file
> to find a list of maintainers/interested parties.

Sounds like a good idea; I'll give it a shot over the next few days, unless
someone already has one :)

> Imagine a patch against devfs..
>
> $ cclist my.devfs.patch.diff
> Richard Gooch <[email protected]>
> Alexander Viro <[email protected]>

I'd add one level of abstraction: have each filename map to a "module" name.
In this case, each filename relating to devfs would map to module "devfs";
there would then be an entry mapping devfs <-> Richard.

(Perhaps a hierarchy - fs.devfs - with people like Al listed for "fs"?)

> This 'little black book of addresses' doesn't have to be anything
> wonderful, but its tedious work for someone to make the textfile
> mapping the various source files to email addresses.

It should be a little easier having a mapping to a module - in most cases,
there's a clear "module" to which each file belongs. Then just track who's
"subscribed to" that module...

James.

2001-12-27 23:32:28

[permalink] [raw]

Subject: Re: [RFC][PATCH] unchecked request_region's in drivers/net

On Dec 27, 2001 23:44 +0100, Dave Jones wrote:
> Someone with far too much time on their hands would be my personal
> hero[*] if they were to write a script (in language of their choice) to
> parse a diff, extract filename, and do lookup in a flat text file
> to find a list of maintainers/interested parties.
>
> Imagine a patch against devfs..
>
> $ cclist my.devfs.patch.diff
> Richard Gooch <[email protected]>
> Alexander Viro <[email protected]>
>
> SCNR 8-)
>
> This 'little black book of addresses' doesn't have to be anything
> wonderful, but its tedious work for someone to make the textfile
> mapping the various source files to email addresses.
>
> Someone (Alan?) suggested having something like a web interface
> allowing anyone interested in any particular file to register
> their interest, and get added to the cclist for that file.
> Which is also a cool idea.

Well, in the past I had suggested to ESR (and he agreed) that it would
be nice to split up the MAINTAINERS file (and maybe even Configure.help)
to be more heirarchical in nature, so that there would be a MAINTAINER
file in each directory, and maybe even MAINTAINER.<file> for files in
common directories like drivers/net/foo.c. In the top-level MAINTAINER
file would only be something like "Marcello Tosatti" to cover the
entire tree, and e.g. "David Miller" in the net/MAINTAINER, "Al Viro" in
the fs/MAINTAINER, "Stephen Tweedie" in fs/ext3/MAINTAINER, etc.

This way the entire tree has coverage, and if a high-level maintainer gets
email for stuff that isn't relevant to them then they create a sub-MAINTAINER
file for the things they want to delegate. It could be possible to add a
keyword to the file which says "I have full authority for this tree and don't
include a higher-level maintainer in cclist output". This would mean that
for people like DaveM or arch maintainers who want ALL patches to go through
them, it would not list Marcello or Linus in the cclist. For "unmaintained"
areas (e.g. drivers) the only person listed would be the top-level maintainer,
and it would be easy for him to say "you are the first person to care about
this driver in a while, you are now the maintainer".

It might be good (for compatibility sake) to build the top-level MAINTAINERS
file from all of the sub-MAINTAINER files in the tree.

This approach doesn't have the "scalability" of the web-based subscription
model, but it _does_ have the benefit that it is kept in-sync with the tree
that it is distributed with, and it would be a simple script to determine
whom to send patches to (i.e. the proposed "cclist" script above would be
very easy to write and could be CLI only). I would think the two are
complimentary.

Cheers, Andreas
--
Andreas Dilger
http://sourceforge.net/projects/ext2resize/
http://www-mddsp.enel.ucalgary.ca/People/adilger/

2001-12-27 23:40:38

[permalink] [raw]

Subject: Re: [RFC][PATCH] unchecked request_region's in drivers/net

Em Thu, Dec 27, 2001 at 04:31:30PM -0700, Andreas Dilger escreveu:
> On Dec 27, 2001 23:44 +0100, Dave Jones wrote:
>
> Well, in the past I had suggested to ESR (and he agreed) that it would
> be nice to split up the MAINTAINERS file (and maybe even Configure.help)

this already happens for the net/ directory to some degree, look at
net/README, the problem, as with MAINTAINERS, is that is way outdated,
listing Alan, for example, as the maintainer for net/core...

- Arnaldo

2001-12-27 23:42:10

[permalink] [raw]

Subject: Re: [RFC][PATCH] unchecked request_region's in drivers/net

On Thu, 27 Dec 2001, James A Sutherland wrote:

> I'd add one level of abstraction: have each filename map to a "module" name.
> In this case, each filename relating to devfs would map to module "devfs";
> there would then be an entry mapping devfs <-> Richard.
> (Perhaps a hierarchy - fs.devfs - with people like Al listed for "fs"?)

Could work, but there are some bits that have no maintainer as such,
eg, the pci irq routing. There have been several interested parties
hacking on it however. A means of having people add themselves to the
list would be a 'must have' feature.

> It should be a little easier having a mapping to a module - in most cases,
> there's a clear "module" to which each file belongs. Then just track who's
> "subscribed to" that module...

Having a mapping from kernel source filename -> email address would
still be preferred personally.

$ cclist devfs

is really not much of an improvement over

$ grep -C3 -i devfs MAINTAINERS

Other than the addition of extra 'interested, cc me too' people.

The only problem with my original idea is that its a pita to keep
up to date. kernel files get added and removed on a weekly (sometimes
daily) basis. Whoever is dumb^Wwonderful enough to volunteer to
maintain such a database is likely to have their work cut out for
them, so maybe your module idea is preferable.

Hmmm..

Dave.

--
| Dave Jones. http://www.codemonkey.org.uk
| SuSE Labs

2001-12-27 23:52:20

[permalink] [raw]

Subject: Re: [RFC][PATCH] unchecked request_region's in drivers/net

On Thu, 27 Dec 2001, Andreas Dilger wrote:

> common directories like drivers/net/foo.c. In the top-level MAINTAINER
> file would only be something like "Marcello Tosatti" to cover the
> entire tree, and e.g. "David Miller" in the net/MAINTAINER, "Al Viro" in
> the fs/MAINTAINER, "Stephen Tweedie" in fs/ext3/MAINTAINER, etc.

Indeed, this solves the keeping the list up to date with whats
in the tree, and has provision for arbitary numbers of METOO:
fields (Though that could get messy, whats to stop Linus getting bombed
with a zillion additions from people who just want to get their
name in a kernel tarball for free). One way could be approval from
the actual maintainer "Yes, he's sent lots of patches, add him to
METOO:".

It does seem a little messy having them scattered all over the tree
too. Extending top-level MAINTAINERS to add necessary fields could
also be done I suppose.

Dave.

--
| Dave Jones. http://www.codemonkey.org.uk
| SuSE Labs

2001-12-28 00:12:34

[permalink] [raw]

Subject: Re: [RFC][PATCH] unchecked request_region's in drivers/net

On Dec 27, 2001 21:40 -0200, Arnaldo Carvalho de Melo wrote:
> Em Thu, Dec 27, 2001 at 04:31:30PM -0700, Andreas Dilger escreveu:
> > Well, in the past I had suggested to ESR (and he agreed) that it would
> > be nice to split up the MAINTAINERS file (and maybe even Configure.help)
>
> this already happens for the net/ directory to some degree, look at
> net/README, the problem, as with MAINTAINERS, is that is way outdated,
> listing Alan, for example, as the maintainer for net/core...

Well, if the MAINTAINERS file isn't up-to-date (which I know it isn't,
see "Remy Card" as ext2 maintainer) then there isn't much that can be
done by users/developers to submit patches to the right people at all.
I think part of the problem is Linus' hesitance to be authoritarian and
add/remove people from the MAINTAINERS list if they don't specifically
ask for the change (and submit a patch to that effect).

Yes, long time readers of l-k know that Dave (and Alexy) is the contact
for net stuff, and Al is the contact for VFS stuff, but nobody else does.
Also, how many emails (in addition to my own) were sent to Remy Card for
ext2 fixes (who is even listed with "Maintained" after his name), yet in
the entire time I've been working on Linux I have never seen a single
email from him (in private or public). Urgh, Remy is also listed in the
under the ext3 entry, how did _that_ happen Andrew? MAINTAINERS != CREDITS

Someone (sadly not me) needs to take over MAINTAINERS like ESR took over
Configure.help and "ping" all of the maintainers that haven't been heard
from and either do some searching for new email addresses, or delete them
entirely from the file. This may ruffle a few feathers, but in the end it
will just be documenting what "the old boys' network of l-k" already knows
and does for patch submission today.

Having a heirarchical MAINTAINER file as I proposed also means:
a) It is closer to the code in question, and _someone_ will be on the hook
for the code in that tree, at least until they delegate it elsewhere.
The descriptions in the current MAINTAINERS file are sometimes hard to
link to specific files if you don't really understand what is going on.
b) It is easier to update more frequently because there are less chances
of patch conflicts (minor issue I know).
c) It is easier to keep external patchsets, as they can have their own
MAINTAINER (and Configure.help) within the tree.

As for "interested parties", this could rather easily be handled by adding
an entry to the MAINTAINER for various parts of the heirarchy which is a
mailing list (e.g. [email protected] in mm/MAINTAINER, linux-scsi@...
in drivers/scsi/MAINTAINER, etc). These would also be listed by the
hypothetical "cclist" tool included with the kernel.

Cheers, Andreas
--
Andreas Dilger
http://sourceforge.net/projects/ext2resize/
http://www-mddsp.enel.ucalgary.ca/People/adilger/

2001-12-28 00:15:33

[permalink] [raw]

Subject: Re: [RFC][PATCH] unchecked request_region's in drivers/net

On Dec 28, 2001 00:51 +0100, Dave Jones wrote:
> On Thu, 27 Dec 2001, Andreas Dilger wrote:
> > common directories like drivers/net/foo.c. In the top-level MAINTAINER
> > file would only be something like "Marcello Tosatti" to cover the
> > entire tree, and e.g. "David Miller" in the net/MAINTAINER, "Al Viro" in
> > the fs/MAINTAINER, "Stephen Tweedie" in fs/ext3/MAINTAINER, etc.
>
> Indeed, this solves the keeping the list up to date with whats
> in the tree, and has provision for arbitary numbers of METOO:
> fields (Though that could get messy, whats to stop Linus getting bombed
> with a zillion additions from people who just want to get their
> name in a kernel tarball for free). One way could be approval from
> the actual maintainer "Yes, he's sent lots of patches, add him to
> METOO:".

I'd rather avoid having Linus become a human mailing-list administrator.

Rather, the "METOO" section would be handled by just adding a mailing
list address in the relevant MAINTAINER file. The above examples would
be covered by linux-kernel, linux-net, linux-fsdevel, and ext2-devel.

Cheers, Andreas
--
Andreas Dilger
http://sourceforge.net/projects/ext2resize/
http://www-mddsp.enel.ucalgary.ca/People/adilger/

2001-12-28 00:25:35

[permalink] [raw]

Subject: Re: [RFC][PATCH] unchecked request_region's in drivers/net

Em Thu, Dec 27, 2001 at 05:11:30PM -0700, Andreas Dilger escreveu:
> On Dec 27, 2001 21:40 -0200, Arnaldo Carvalho de Melo wrote:
> > this already happens for the net/ directory to some degree, look at
> > net/README, the problem, as with MAINTAINERS, is that is way outdated,
> > listing Alan, for example, as the maintainer for net/core...
>
> Well, if the MAINTAINERS file isn't up-to-date (which I know it isn't,
> see "Remy Card" as ext2 maintainer) then there isn't much that can be
> done by users/developers to submit patches to the right people at all.
> I think part of the problem is Linus' hesitance to be authoritarian and
> add/remove people from the MAINTAINERS list if they don't specifically
> ask for the change (and submit a patch to that effect).

Yup, Linus told this in a recent thread (this one?), and Al seems still not
interested in having his name there, its his right, but the fact is that he
_is_ the one doing maintainance for the VFS, so I don't think that putting
his name there would be something bad for him or whatever, but hey, if
Linus isn't authoritarian wrt this and Al doesn't want to have his name
listed there, what can we do? Ditto for other de-facto maintainers that are
not listed there.

And BTW, a janitor did something like: look at maintainers? nothing there?
send a message to all the addresses in the source file touched, what did he
got? some responses, yes, but _tons_ of bounces...

- Arnaldo

2001-12-28 00:33:54

by Alan

[permalink] [raw]

Subject: Re: [RFC][PATCH] unchecked request_region's in drivers/net

> Rather, the "METOO" section would be handled by just adding a mailing
> list address in the relevant MAINTAINER file. The above examples would
> be covered by linux-kernel, linux-net, linux-fsdevel, and ext2-devel.

99% of the problem can IMHO be solved by notifying after the event since
we have pre-patches and stuff can be handled after a pre anyway. Then
its

grep diff for filenames
mail anyone whose regexp matched the list of filename matches

Alan

2001-12-28 09:49:57

by James A Sutherland

[permalink] [raw]

Subject: Re: [RFC][PATCH] unchecked request_region's in drivers/net

On Thursday 27 December 2001 11:40 pm, Dave Jones wrote:
> On Thu, 27 Dec 2001, James A Sutherland wrote:
> > I'd add one level of abstraction: have each filename map to a "module"
> > name. In this case, each filename relating to devfs would map to module
> > "devfs"; there would then be an entry mapping devfs <-> Richard.
> > (Perhaps a hierarchy - fs.devfs - with people like Al listed for "fs"?)
>
> Could work, but there are some bits that have no maintainer as such,
> eg, the pci irq routing. There have been several interested parties
> hacking on it however. A means of having people add themselves to the
> list would be a 'must have' feature.

Yes. Something like mailman's approach, perhaps? (You set a password when you
subscribe, then you can see and change which modules you are subscribed to
using that password.)

> > It should be a little easier having a mapping to a module - in most
> > cases, there's a clear "module" to which each file belongs. Then just
> > track who's "subscribed to" that module...
>
> Having a mapping from kernel source filename -> email address would
> still be preferred personally.

You would still have that feature: my script was intended to take patches,
filenames or modulenames as input, and list the interested parties and/or
module names.

So...

$ cclist -m big_patch
fs/devfs
arch/x86

$ cclist -f fs/devfs/base.c
Richard Gooch <...>
Al Viro <...>

> $ cclist devfs
>
> is really not much of an improvement over
>
> $ grep -C3 -i devfs MAINTAINERS
>
> Other than the addition of extra 'interested, cc me too' people.

And tracking of filename<->module relationships, allowing you to look up who
would be interested in a given patch.

> The only problem with my original idea is that its a pita to keep
> up to date. kernel files get added and removed on a weekly (sometimes
> daily) basis. Whoever is dumb^Wwonderful enough to volunteer to
> maintain such a database is likely to have their work cut out for
> them, so maybe your module idea is preferable.

In the way I'm suggesting, it just simplifies matters a little: rather than
having to list RG and AV separately for every single file in devfs, just say
"this new file is part of devfs" and it "inherits" them that way.

James.

2001-12-28 11:15:02

[permalink] [raw]

Subject: Re: [RFC][PATCH] unchecked request_region's in drivers/net

On Fri, 28 Dec 2001, James A Sutherland wrote:

> In the way I'm suggesting, it just simplifies matters a little: rather than
> having to list RG and AV separately for every single file in devfs, just say
> "this new file is part of devfs" and it "inherits" them that way.

gotcha, sounds ok to me, still quite a bit of work to build the database
needed for this, but as several others have pointed out, this would save
so much time for anyone who sends out lots of patches.

Dave.

--
| Dave Jones. http://www.codemonkey.org.uk
| SuSE Labs

2001-12-28 18:35:46

by Riley Williams

[permalink] [raw]

Subject: Re: [RFC][PATCH] unchecked request_region's in drivers/net

Hi Andreas.

>> Someone (Alan?) suggested having something like a web interface
>> allowing anyone interested in any particular file to register their
>> interest, and get added to the cclist for that file. Which is also
>> a cool idea.

> Well, in the past I had suggested to ESR (and he agreed) that it
> would be nice to split up the MAINTAINERS file (and maybe even
> Configure.help) to be more heirarchical in nature, so that there
> would be a MAINTAINER file in each directory, and maybe even
> MAINTAINER.<file> for files in common directories like
> drivers/net/foo.c. In the top-level MAINTAINER file would only be
> something like "Marcello Tosatti" to cover the entire tree, and e.g.
> "David Miller" in the net/MAINTAINER, "Al Viro" in the
> fs/MAINTAINER, "Stephen Tweedie" in fs/ext3/MAINTAINER, etc.

Can I suggest an alternative: Retain the MAINTAINERS file as it
currently is, but add a PATCHES-TO file in each subdirectory that
states how to handle patches relating to that directory, and have
these files follow a strict format, possibly...

===8<=== CUT ===>8===

HomeDir = linux/subsystem/
List = [email protected]
Maintainer = Guess Who <[email protected]>
Watch * = Interested <[email protected]>
Watch PATCHES-TO = John Doa <[email protected]>

===8<=== CUT ===>8===

...where the lines are as follows...

HOMEDIR = relative-path

The directory in the tree that this PATCHES-TO
file belongs in. Must occur exactly once, and
must start with linux/ to indicate the base
directory this tree has been installed in.

LIST = mailing-list-email-address

A mailing list dealing with this part of the
kernel tree. Can be repeated if multiple lists
need to be specified, or omitted if there is no
specific mailing list for this subsystem.

NEWS = newsgroup

A newsgroup dealing with this part of the kernel
tree. Can be repeated if multiple newsgroups need
to be specified, or omitted if no newsgroups are
relevant to this section of the tree.

MAINTAINER = name <email-address>

The name and email address of the primary maintainer
for files in this directory. Can be repeated if
multiple maintainers need to be specified. Must
occur in the file in the base directory, but can
be omitted in any subdirectory in which case it
indicates that the maintainer of the parent also
maintains this subdirectory.

WATCH filespec = name <email-address>

Specifies that the email address specified is also
interested in patches relating to the specified
files, and should be CC'd patches relating to just
the files specified. Files of interest are selected
using ls style wildcards. Can be repeated as often
as required for either the same or different files.
The filespec can not contain / characters, and only
matches files in the current directory.

...and all unrecognised lines are taken to be valid comments and are
ignored by all tools.

Comments?

Best wishes from Riley.

2001-12-28 18:39:56

by Alan

[permalink] [raw]

Subject: Re: [RFC][PATCH] unchecked request_region's in drivers/net

> currently is, but add a PATCHES-TO file in each subdirectory that
> states how to handle patches relating to that directory, and have
> these files follow a strict format, possibly...

Add the patches to to the maintainers as another field. If the patches
go to someone who isnt claiming to be a maintainer something is wrong

2001-12-28 19:39:31

by Rik van Riel

[permalink] [raw]

Subject: Re: [RFC][PATCH] unchecked request_region's in drivers/net

On Fri, 28 Dec 2001, Alan Cox wrote:

> > currently is, but add a PATCHES-TO file in each subdirectory that
> > states how to handle patches relating to that directory, and have
> > these files follow a strict format, possibly...
>
> Add the patches to to the maintainers as another field. If the patches
> go to someone who isnt claiming to be a maintainer something is wrong

We'll have to educate viro, then. I bet he won't like it
if VFS patches don't get send to him, but he's not listed
as a maintainer ...

Rik
--
DMCA, SSSCA, W3C? Who cares? http://thefreeworld.net/

http://www.surriel.com/ http://distro.conectiva.com/

2001-12-28 20:36:10