* Re: [KJ] [PATCH V2] Check return code for IBM Olympic /proc entry
2005-05-18 8:21 [KJ] [PATCH V2] Check return code for IBM Olympic /proc entry Kirk True
@ 2005-05-25 13:07 ` Domen Puncer
2005-05-26 6:22 ` Kirk True
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Domen Puncer @ 2005-05-25 13:07 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 1489 bytes --]
On 18/05/05 01:21 -0700, Kirk True wrote:
> The IBM Olympic driver assumes that the call to create_proc_read_entry
> always succeeds. This patch simply logs appropriately whether in
> actuality it was created or not.
>
> This falls under the 'Audit return codes (and handle failure correctly)
> for create_proc_*()' item on the TODO list. Also added KERN_* constants
> as per Christophe Lucas' recommendation.
>
> Signed-off-by: Kirk True <kirk@kirkandsheila.com>
^^ please don't
>
> --- linux-2.6.12-rc4-kj/drivers/net/tokenring/olympic.c 2005-05-17 23:31:14.000000000 -0700
> +++ linux-2.6.12-rc4-kj-debug/drivers/net/tokenring/olympic.c 2005-05-18 01:11:00.000000000 -0700
> @@ -267,11 +267,16 @@ static int __devinit olympic_probe(struc
> register_netdev(dev) ;
> printk("Olympic: %s registered as: %s\n",olympic_priv->olympic_card_name,dev->name);
> if (olympic_priv->olympic_network_monitor) { /* Must go after register_netdev as we need the device name */
> + struct proc_dir_entry *proc_dir_entry;
> char proc_name[20] ;
> strcpy(proc_name,"net/olympic_") ;
> strcat(proc_name,dev->name) ;
> - create_proc_read_entry(proc_name,0,NULL,olympic_proc_info,(void *)dev) ;
> - printk("Olympic: Network Monitor information: /proc/%s\n",proc_name);
> + proc_dir_entry = create_proc_read_entry(proc_name,0,NULL,olympic_proc_info,(void *)dev) ;
I removed the unneeded (void*) cast (couldn't resist) and trailing
whitespace here.
Thanks,
Domen
[-- Attachment #2: Type: text/plain, Size: 167 bytes --]
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [KJ] [PATCH V2] Check return code for IBM Olympic /proc entry
2005-05-18 8:21 [KJ] [PATCH V2] Check return code for IBM Olympic /proc entry Kirk True
2005-05-25 13:07 ` Domen Puncer
@ 2005-05-26 6:22 ` Kirk True
2005-05-26 6:34 ` Domen Puncer
2005-05-26 6:49 ` Kirk True
3 siblings, 0 replies; 5+ messages in thread
From: Kirk True @ 2005-05-26 6:22 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 1935 bytes --]
Hi Domen,
On Wed, 2005-05-25 at 15:07 +0200, Domen Puncer wrote:
> On 18/05/05 01:21 -0700, Kirk True wrote:
> > The IBM Olympic driver assumes that the call to create_proc_read_entry
> > always succeeds. This patch simply logs appropriately whether in
> > actuality it was created or not.
> >
> > This falls under the 'Audit return codes (and handle failure correctly)
> > for create_proc_*()' item on the TODO list. Also added KERN_* constants
> > as per Christophe Lucas' recommendation.
> >
> > Signed-off-by: Kirk True <kirk@kirkandsheila.com>
> ^^ please don't
Sorry to be daft, but "please don't" do what exactly? I tried to follow
the guidelines. My apologies in advance for the error.
> >
> > --- linux-2.6.12-rc4-kj/drivers/net/tokenring/olympic.c 2005-05-17 23:31:14.000000000 -0700
> > +++ linux-2.6.12-rc4-kj-debug/drivers/net/tokenring/olympic.c 2005-05-18 01:11:00.000000000 -0700
> > @@ -267,11 +267,16 @@ static int __devinit olympic_probe(struc
> > register_netdev(dev) ;
> > printk("Olympic: %s registered as: %s\n",olympic_priv->olympic_card_name,dev->name);
> > if (olympic_priv->olympic_network_monitor) { /* Must go after register_netdev as we need the device name */
> > + struct proc_dir_entry *proc_dir_entry;
> > char proc_name[20] ;
> > strcpy(proc_name,"net/olympic_") ;
> > strcat(proc_name,dev->name) ;
> > - create_proc_read_entry(proc_name,0,NULL,olympic_proc_info,(void *)dev) ;
> > - printk("Olympic: Network Monitor information: /proc/%s\n",proc_name);
> > + proc_dir_entry = create_proc_read_entry(proc_name,0,NULL,olympic_proc_info,(void *)dev) ;
>
> I removed the unneeded (void*) cast (couldn't resist) and trailing
> whitespace here.
I didn't do too much beyond the specific request to handle the audit
code. I'm coming from the 'try to change it as little as possible
because you don't know what you're doing just yet' mindset :)
Thanks,
Kirk
[-- Attachment #2: Type: text/plain, Size: 167 bytes --]
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [KJ] [PATCH V2] Check return code for IBM Olympic /proc entry
2005-05-18 8:21 [KJ] [PATCH V2] Check return code for IBM Olympic /proc entry Kirk True
2005-05-25 13:07 ` Domen Puncer
2005-05-26 6:22 ` Kirk True
@ 2005-05-26 6:34 ` Domen Puncer
2005-05-26 6:49 ` Kirk True
3 siblings, 0 replies; 5+ messages in thread
From: Domen Puncer @ 2005-05-26 6:34 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 1638 bytes --]
On 25/05/05 23:22 -0700, Kirk True wrote:
> Hi Domen,
>
> On Wed, 2005-05-25 at 15:07 +0200, Domen Puncer wrote:
> > On 18/05/05 01:21 -0700, Kirk True wrote:
> > > The IBM Olympic driver assumes that the call to create_proc_read_entry
> > > always succeeds. This patch simply logs appropriately whether in
> > > actuality it was created or not.
> > >
> > > This falls under the 'Audit return codes (and handle failure correctly)
> > > for create_proc_*()' item on the TODO list. Also added KERN_* constants
> > > as per Christophe Lucas' recommendation.
> > >
> > > Signed-off-by: Kirk True <kirk@kirkandsheila.com>
> > ^^ please don't
>
> Sorry to be daft, but "please don't" do what exactly? I tried to follow
> the guidelines. My apologies in advance for the error.
Put spaces before "Signed-off-by" :-)
Now that check SubmittingPatches, I see it's there too.
Still, most of patches don't do it, and I imagine it can break scripts.
> > > - create_proc_read_entry(proc_name,0,NULL,olympic_proc_info,(void *)dev) ;
> > > - printk("Olympic: Network Monitor information: /proc/%s\n",proc_name);
> > > + proc_dir_entry = create_proc_read_entry(proc_name,0,NULL,olympic_proc_info,(void *)dev) ;
> >
> > I removed the unneeded (void*) cast (couldn't resist) and trailing
> > whitespace here.
>
> I didn't do too much beyond the specific request to handle the audit
> code. I'm coming from the 'try to change it as little as possible
> because you don't know what you're doing just yet' mindset :)
Well... stripping trailing whitespace is not too much to ask.
Anyway, patch should be in next -kj. Thanks.
Domen
[-- Attachment #2: Type: text/plain, Size: 167 bytes --]
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [KJ] [PATCH V2] Check return code for IBM Olympic /proc entry
2005-05-18 8:21 [KJ] [PATCH V2] Check return code for IBM Olympic /proc entry Kirk True
` (2 preceding siblings ...)
2005-05-26 6:34 ` Domen Puncer
@ 2005-05-26 6:49 ` Kirk True
3 siblings, 0 replies; 5+ messages in thread
From: Kirk True @ 2005-05-26 6:49 UTC (permalink / raw)
To: kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 1958 bytes --]
On Thu, 2005-05-26 at 08:34 +0200, Domen Puncer wrote:
> On 25/05/05 23:22 -0700, Kirk True wrote:
> > Hi Domen,
> >
> > On Wed, 2005-05-25 at 15:07 +0200, Domen Puncer wrote:
> > > On 18/05/05 01:21 -0700, Kirk True wrote:
> > > > The IBM Olympic driver assumes that the call to create_proc_read_entry
> > > > always succeeds. This patch simply logs appropriately whether in
> > > > actuality it was created or not.
> > > >
> > > > This falls under the 'Audit return codes (and handle failure correctly)
> > > > for create_proc_*()' item on the TODO list. Also added KERN_* constants
> > > > as per Christophe Lucas' recommendation.
> > > >
> > > > Signed-off-by: Kirk True <kirk@kirkandsheila.com>
> > > ^^ please don't
> >
> > Sorry to be daft, but "please don't" do what exactly? I tried to follow
> > the guidelines. My apologies in advance for the error.
>
> Put spaces before "Signed-off-by" :-)
> Now that check SubmittingPatches, I see it's there too.
> Still, most of patches don't do it, and I imagine it can break scripts.
Oh. OK. No problem. I thought I'd seen it that way in other patches.
I'll have to remember that :) Thanks.
>
> > > > - create_proc_read_entry(proc_name,0,NULL,olympic_proc_info,(void *)dev) ;
> > > > - printk("Olympic: Network Monitor information: /proc/%s\n",proc_name);
> > > > + proc_dir_entry = create_proc_read_entry(proc_name,0,NULL,olympic_proc_info,(void *)dev) ;
> > >
> > > I removed the unneeded (void*) cast (couldn't resist) and trailing
> > > whitespace here.
> >
> > I didn't do too much beyond the specific request to handle the audit
> > code. I'm coming from the 'try to change it as little as possible
> > because you don't know what you're doing just yet' mindset :)
>
> Well... stripping trailing whitespace is not too much to ask.
> Anyway, patch should be in next -kj. Thanks.
OK. I'll try to clean up as much as I can in a given area.
Thanks for the feedback!
Kirk
[-- Attachment #2: Type: text/plain, Size: 167 bytes --]
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 5+ messages in thread