All of lore.kernel.org
 help / color / mirror / Atom feed
* self-assignment in dino_card_setup
@ 2015-12-29 20:03 Rasmus Villemoes
  2015-12-29 20:22 ` Helge Deller
  0 siblings, 1 reply; 3+ messages in thread
From: Rasmus Villemoes @ 2015-12-29 20:03 UTC (permalink / raw)
  To: James E.J. Bottomley, Helge Deller; +Cc: linux-parisc

Line 474 of drivers/parisc/dino.c reads

        res->name = dino_dev->hba.lmmio_space.name;

but res is &dino_dev->hba.lmmio_space, so this seems to just assign NULL
again to res->name. Presumably the allocation of <= 128 bytes never
fails, but I wonder what the purpose was, and if the confusing else
branch couldn't just be deleted (and then one should just do res->name =
kmemdup(name, size+1, GFP_KERNEL); and delete the if as well).

Rasmus

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: self-assignment in dino_card_setup
  2015-12-29 20:03 self-assignment in dino_card_setup Rasmus Villemoes
@ 2015-12-29 20:22 ` Helge Deller
  2015-12-29 20:51   ` James Bottomley
  0 siblings, 1 reply; 3+ messages in thread
From: Helge Deller @ 2015-12-29 20:22 UTC (permalink / raw)
  To: Rasmus Villemoes, James E.J. Bottomley; +Cc: linux-parisc

Hi Rasmus,

On 29.12.2015 21:03, Rasmus Villemoes wrote:
> Line 474 of drivers/parisc/dino.c reads
> 
>         res->name = dino_dev->hba.lmmio_space.name;
> 
> but res is &dino_dev->hba.lmmio_space, so this seems to just assign NULL
> again to res->name. Presumably the allocation of <= 128 bytes never
> fails, but I wonder what the purpose was, and if the confusing else
> branch couldn't just be deleted (and then one should just do res->name =
> kmemdup(name, size+1, GFP_KERNEL); and delete the if as well).

I'd suggest to just delete the else branch.
Do you want to send a patch with signed-off ?

Helge


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: self-assignment in dino_card_setup
  2015-12-29 20:22 ` Helge Deller
@ 2015-12-29 20:51   ` James Bottomley
  0 siblings, 0 replies; 3+ messages in thread
From: James Bottomley @ 2015-12-29 20:51 UTC (permalink / raw)
  To: Helge Deller, Rasmus Villemoes, James E.J. Bottomley; +Cc: linux-parisc

On Tue, 2015-12-29 at 21:22 +0100, Helge Deller wrote:
> Hi Rasmus,
> 
> On 29.12.2015 21:03, Rasmus Villemoes wrote:
> > Line 474 of drivers/parisc/dino.c reads
> > 
> >         res->name = dino_dev->hba.lmmio_space.name;
> > 
> > but res is &dino_dev->hba.lmmio_space, so this seems to just assign
> > NULL
> > again to res->name. Presumably the allocation of <= 128 bytes never
> > fails, but I wonder what the purpose was, and if the confusing else
> > branch couldn't just be deleted (and then one should just do res
> > ->name =
> > kmemdup(name, size+1, GFP_KERNEL); and delete the if as well).
> 
> I'd suggest to just delete the else branch.
> Do you want to send a patch with signed-off ?

Actually, I'd guess the intention was to put back whatever the contents
of lmmio_space.name originally was ... it's usually some default static
string.

James



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-12-29 20:51 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-29 20:03 self-assignment in dino_card_setup Rasmus Villemoes
2015-12-29 20:22 ` Helge Deller
2015-12-29 20:51   ` James Bottomley

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.