Re: [dpdk-dev] [PATCH v2 06/11] app/test: use linked list to store PCI drivers

[Jan Viktorin <viktorin@rehivetech.com>]

On Thu, 12 May 2016 18:08:16 +0200
Thomas Monjalon <thomas.monjalon@6wind.com> wrote:

> 2016-05-12 17:53, Jan Viktorin:
> > On Thu, 12 May 2016 17:31:28 +0200
> > Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
> >   
> > > 2016-05-10 20:13, Jan Viktorin:  
> > > > The test unregisters all real drivers before starting into an array. This
> > > > inflexiable as we can use a linked list for this purpose.    
> > > 
> > > I don't understand this. Maybe some words are missing.  
> > 
> > Better?
> > 
> > The test unregisters all real drivers before starting (stored into an array).
> > This is inflexiable (due to its fixed size) and we can use a linked list for
> > this purpose.    
> 
> Better with a past tense? "The test was unregistering..."

No. This patch does not change the semantics (well, it does, because
the list of drivers become global but that's all). The test still
unregisters the drivers. That is the important fact to consider.

Perhaps:

The test unregisters all drivers before start. The drivers were stored
into a fixed-sized array. This is inflexible. This patch change this to
utilize a linked list for the same purpose.

> 
> > > > +/* real drivers (not used for testing) */    
> > > 
> > > What do mean by "not used for testing"?  
> > 
> > The test now avoids the DPDK builtin drivers. It only considers its
> > internal fake drivers my_driver and my_driver2. So the real drivers
> > are temporarily store into the real_pci_driver_list and returned back
> > after the test finishes.  
> 
> Maybe adding "Save" or "Backup" would make it clear.

Ok, makes sense.

> 
> > It is the linked list mentioned in the commit log. It replaces the
> > original fixed-size array.
> > 
> > (For drivers, it does not matter that much. But for devices, I think,
> > it is not a good practice to consider them in autotests. Every PC
> > where you execute the tests have different set of PCI devices.)  
> 
> Yes

Thanks for comments!

Jan

-- 
  Jan Viktorin                E-mail: Viktorin@RehiveTech.com
  System Architect            Web:    www.RehiveTech.com
  RehiveTech
  Brno, Czech Republic