Re: [PATCH 3/4] kni: replace strict_strtoul with kstrtoul

[Jincheng Miao <jmiao-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

----- Original Message -----
> Hi Jincheng,
> 
> 2014-12-10 11:33, Jincheng Miao:
> > From upstream kernel commit 3db2e9cd, strict_strto* serial functions
> > are removed. So that we should directly used kstrtoul instead.
> > 
> > And add kni/compat.h for be compatible with older kernel.
> > 
> > Signed-off-by: Jincheng Miao <jmiao-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> [...]
> > new file mode 100644
> > index 0000000..c8c662c
> > --- /dev/null
> > +++ b/lib/librte_eal/linuxapp/kni/compat.h
> > @@ -0,0 +1,16 @@
> > +/*
> > + * Minimal wrappers to allow compiling kni on older kernels.
> > + */
> > +
> > +#ifndef RHEL_RELEASE_VERSION
> > +#define RHEL_RELEASE_VERSION(a, b) (((a) << 8) + (b))
> > +#endif
> > +
> > +#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 39) && \
> > +        (!(defined(RHEL_RELEASE_CODE) && \
> > +           RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(6, 4)))
> 
> The first indent character must be a tab (highlighted by checkpatch).

Yes, I think the TAB is replaced by my vim :(

> 
> [...]
> > --- a/lib/librte_eal/linuxapp/kni/kni_vhost.c
> > +++ b/lib/librte_eal/linuxapp/kni/kni_vhost.c
> > @@ -740,7 +740,7 @@ set_sock_en(struct device *dev, struct device_attribute
> > *attr,
> >  	unsigned long en;
> >  	int err = 0;
> >  
> > -	if (0 != strict_strtoul(buf, 0, &en))
> > +	if (0 != kstrtoul(buf, 0, &en))
> >  		return -EINVAL;
> >  
> >  	if (en)
> 
> It seems you forgot to include the new compat.h.
> 
> Did you do some tests with different Fedora/RHEL versions?

Yes, missing compat.h in kni_vhost.c.


And, I want to get your opinion about adding compat.h to kni and xen_dom0.
The pros: easy to implement and minimal wrapper for older kernel.
The cons: there is so many compat.h, and the file kcompat.h also makes user confuse.


> 
> Thanks
> --
> Thomas
>