Re: [Outreachy kernel] [PATCH v2] Staging: sm750fb: ddk750_help: Included KERN_facility level using dev_info

[Sudip Mukherjee <sudipm.mukherjee@gmail.com>]

On Sat, Oct 10, 2015 at 09:24:52AM +0200, Julia Lawall wrote:
> On Sat, 10 Oct 2015, Sudip Mukherjee wrote:
> 
> > On Fri, Oct 09, 2015 at 07:51:18PM +0530, Shivani Bhardwaj wrote:
> > > Replaced the function printk() with dev_info() to fix the warning
> > > generated by checkpatch : printk() should include KERN_ facility level.
> > > 
> > > Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
> > > ---
> > > Changes in v2:
> > > 	-Replaced the function pr_info() with dev_info()
> > > 
> > >  drivers/staging/sm750fb/ddk750_help.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/staging/sm750fb/ddk750_help.c b/drivers/staging/sm750fb/ddk750_help.c
> > > index 9637dd3..b877704 100644
> > > --- a/drivers/staging/sm750fb/ddk750_help.c
> > > +++ b/drivers/staging/sm750fb/ddk750_help.c
> > > @@ -1,5 +1,7 @@
> > >  #include "ddk750_help.h"
> > > +#include <linux/pci.h>
> > >  
> > > +static struct pci_dev *pdev;
> > >  void __iomem *mmio750;
> > >  char revId750;
> > >  unsigned short devId750;
> > > @@ -11,7 +13,7 @@ void ddk750_set_mmio(void __iomem *addr, unsigned short devId, char revId)
> > >  	devId750 = devId;
> > >  	revId750 = revId;
> > >  	if (revId == 0xfe)
> > > -		printk("found sm750le\n");
> > > +		dev_info(&pdev->dev, "found sm750le\n");
> > 
> > OOPS .. no..
> > pdev is initialized to NULL, so when you do pdev->dev, you are doing a
> > dereference of NULL pointer.
> 
> To be precise, Shivani's code isn't doing a dereference, it's doing an 
> addition.  But then dev_info will dereference the result of that addition 
> (which is not likely to be NULL, but rather a small integer), and then 
> there will be a problem.

Yes, dereference was wrong word. It is taking the address of that
member. But my main point was that pdev is not initialized. So using
&pdev->dev for dev_info is wrong.

> 
> So one would have to use pr_info here.

pr_info will be better. I can use dev_info here, but the changes that
has to be made for that, I think that will become too complicated for
her, atleast at this stage.

regards
sudip