RE: [PATCH resend] OMAP: DSS2: Replace strncmp() with sysfs_streq() in overlay_manager_store()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Taneja, Archit 
> Sent: Thursday, July 29, 2010 9:16 AM
> To: Premi, Sanjeev; tomi.valkeinen@xxxxxxxxx
> Cc: linux-omap@xxxxxxxxxxxxxxx
> Subject: RE: [PATCH resend] OMAP: DSS2: Replace strncmp() 
> with sysfs_streq() in overlay_manager_store()
> 
> 
> > > > > -----Original Message-----
> > > > > From: linux-omap-owner@xxxxxxxxxxxxxxx 
> > > > > [mailto:linux-omap-owner@xxxxxxxxxxxxxxx] On Behalf Of
> > > > Taneja, Archit
> > > > > Sent: Wednesday, July 28, 2010 11:52 AM
> > > > > To: tomi.valkeinen@xxxxxxxxx
> > > > > Cc: linux-omap@xxxxxxxxxxxxxxx; Taneja, Archit
> > > > > Subject: [PATCH resend] OMAP: DSS2: Replace strncmp() with
> > > > > sysfs_streq() in overlay_manager_store()
> > > > > 
> > > > > In the function overlay_manager_store, the length of the
> > > > string buf is
> > > > > used to comapre the 2 strings, there is a possibility 
> of false 
> > > > > positive match if buf is a prefix string of another manager.
> > > > > 
> > > > > The use of sysfs_streq() fixes this.
> > > > > 
> > > > > Signed-off-by: Archit Taneja <archit@xxxxxx>
> > > > > ---
> > > > >  drivers/video/omap2/dss/overlay.c |    2 +-
> > > > >  1 files changed, 1 insertions(+), 1 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/video/omap2/dss/overlay.c
> > > > > b/drivers/video/omap2/dss/overlay.c
> > > > > index 8233658..244dca8
> > > > > --- a/drivers/video/omap2/dss/overlay.c
> > > > > +++ b/drivers/video/omap2/dss/overlay.c
> > > > > @@ -65,7 +65,7 @@ static ssize_t overlay_manager_store(struct 
> > > > > omap_overlay *ovl, const char *buf,
> > > > >  		for (i = 0; i <
> > > > > omap_dss_get_num_overlay_managers(); ++i) {
> > > > >  			mgr = omap_dss_get_overlay_manager(i);
> > > > >  
> > > > > -			if (strncmp(buf, mgr->name, len) == 0)
> > > > > +			if (sysfs_streq(buf, mgr->name))
> > > > 
> > > > [sp] sysfs_streq() ignores trailing newlines during 
> > comparison. So,
> > > >      the possibility mentioned in the description still stays.
> > > 
> > > The aim is to compare one string which is a sysfs input and 
> > the other 
> > > which is in the kernel.
> > > 
> > > > 
> > > >      I am not familiar with overall context; but 
> wouldn't srtcmp()
> > > >      be the right choice? unless, of course, either of strings
> > > >      being compared are not null terminated.
> > > 
> > > The sysfs input will have a newline and null at the end 
> whereas the 
> > > other string will only have null, strcmp will fail when 
> we want the 
> > > two strings to match.
> > > 
> > > Eg. Sysfs input "lcd\n\0"
> > > Kernel string "lcd\0"
> > > 
> > > strcmp will fail here
> > 
> > [sp] If the patch is intending to fix this condition, then it isn't
> >      evident from the description. You should consider updating the
> >      description.
> 
> The patch isn't intending to fix this condition, this 
> condition doesn't
> occur at all in the existing code. I explained this to tell 
> you why strcmp
> is a bad choice, the patch description tells why sysfs_streq 
> is a better
> choice over strncmp.

[sp] Can you explain the real condition and how/why it can't be handled
     by strcmp()

> 
> > 
> > > 
> > > > 
> > > > >  				break;
> > > > >  
> > > > >  			mgr = NULL;
> > > > > --
> > > > > 1.5.4.7
> > > > > 
> > > > > --
> > > > > To unsubscribe from this list: send the line "unsubscribe
> > > > linux-omap" 
> > > > > in the body of a message to majordomo@xxxxxxxxxxxxxxx More
> > > > majordomo
> > > > > info at  http://vger.kernel.org/majordomo-info.html
> > > > > --
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux