Re: [PATCH v2] x86/earlyprintk: Add a force option for pciserial device

[Thomas Gleixner]

On Tue, 2 Oct 2018, Feng Tang wrote:
> On Tue, Oct 02, 2018 at 11:17:57AM +0200, Thomas Gleixner wrote:
> > On Tue, 2 Oct 2018, Feng Tang wrote:
> > 
> > > Hi Boris,
> > > 
> > > On Mon, Oct 01, 2018 at 10:30:04PM +0200, Borislav Petkov wrote:
> > > > On Mon, Oct 01, 2018 at 10:18:10PM +0800, Feng Tang wrote:
> > > > > As I rechecked, the baud rate for pciserial is optional, so there may
> > > > > be no ",baudrate" following the "force". So this 2 strncmp is to
> > > > > cover conditions for with and without baudrate.
> > > > 
> > > > And what guarantees you have a space after the "force"?
> > > > 
> > > > 	!strncmp(s, "force ", 6)
> > > > 			  ^
> > > 
> > > You are right, it can't be guranteed. Can we still use strncmp(s, "force", 5)
> > > and rely on developer to set it right? any suggestions? thanks,
> > 
> > I don't know why you want strncmp() in the first place. "force" is null
> > terminated already.
> 
> Current code uses: 
> 	earlyprintk=pciserial,bus:device.function[,baudrate]
> with the patch, it will be:
> 	earlyprintk=pciserial,bus:device.function[,force][,baudrate]
> 
> So the force could be followed by ",baudrate".

Sure, but that has nothing to do with strncmp(). The earlyprintk argument
string is tokenized by commata. It really does not matter where the 'force'
string is, but it matters that you check whether it is 'force' and not
'force$RANDOMCHARACTERS'. That's why strncmp() is the wrong thing to do,
because it only compares the first 5 characters and ignores anything
beyond, unless you make sure that the command line part is exactly 5
characters long.

Thanks,

	tglx