Re: [Linux-ATM-General] solos-pci.c: Fix me

[Guy E. <gu...@tr...>]

Hi Chas/Nick,

I think the FIXME is reminder to deal correctly with an unknown command.
At the moment an unknown command is treated as a PKT_COMMAND which is 
incorrect.
I think the correct behaviour would be to ignore unknown commands and 
just free the skb.

That said I doubt this condition ever occurs, which is probably why it 
has been this way since day 1.

Nathan is on vacation at the moment, when he gets back in August I'll 
ask him to tidy this up.

Cheers,
  - Guy.





On 22/07/2014 4:18 AM, chas williams - CONTRACTOR wrote:
> On Mon, 21 Jul 2014 13:42:01 -0400
> Nick Krause <xer...@gm...> wrote:
>
>> On Mon, Jul 21, 2014 at 9:14 AM, chas williams - CONTRACTOR
>> <ch...@cm...> wrote:
> ...
>>> @@ -850,8 +850,7 @@ static void solos_bh(unsigned long card_arg)
>>>                                  dev_kfree_skb_any(skb);
>>>                                  break;
>>>
>>> -                       case PKT_COMMAND:
>>> -                       default: /* FIXME: Not really, surely? */
>>> +                       default: /* PKT_COMMAND */
>>>                                  if (process_command(card, port, skb))
>>>                                          break;power
>>>                                  spin_lock(&card->cli_queue_lock);
>>>
>>> and be done with it since that will preserve existing behavior.
>>
>> My only question then is this function causing bugs as is?
>> Cheers Nick
>>
> It has been there since the first version of this driver was released.
> If it were breaking things, I would have to guess it would have been
> noticed by now.
>
> Just looking at the PKT_* defines, I would wildly guess that PKT_COMMAND
> PKT_POPEN and PKT_PCLOSE are all being handled via the PKT_COMMAND|default
> case.  If not, popen and pclose would be leaking a skb for each usage
> (which would be opening and closing the PVC -- not very often).
>
> popen and pclose just seem like specialized PKT_COMMAND types.
> process_command() woud ignore PKT_POPEN and PKT_PCLOSE since they aren't
> long enough to contain a command.  So maybe the comment should just go
> away.
>
> If you had access to hardware you could probably figure this out pretty
> quickly.  The programmer's manual doesn't seem to be available.
>
> ------------------------------------------------------------------------------
> Want fast and easy access to all the code in your enterprise? Index and
> search up to 200,000 lines of code with a free copy of Black Duck
> Code Sight - the same software that powers the world's largest code
> search on Ohloh, the Black Duck Open Hub! Try it now.
> http://p.sf.net/sfu/bds
> _______________________________________________
> Linux-atm-general mailing list
> Lin...@li...
> https://lists.sourceforge.net/lists/listinfo/linux-atm-general
>


-- 
Guy Ellis
gu...@tr...
www.traverse.com.au
T: +61 3 9386 4435 M: +61 419 398 234