linux-kernel - Re: [PATCH] staging: r8188eu: make dump_chip

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <434188828f98c78d17cef4f62461b75f7197a67f.camel@perches.com>
Date:   Sun, 24 Jul 2022 13:53:59 -0700
From:   Joe Perches <joe@...ches.com>
To:     Philipp Hortmann <philipp.g.hortmann@...il.com>,
        Michael Straube <straube.linux@...il.com>,
        gregkh@...uxfoundation.org
Cc:     Larry.Finger@...inger.net, phil@...lpotter.co.uk,
        linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: r8188eu: make dump_chip_info() static

On Sun, 2022-07-24 at 20:39 +0200, Philipp Hortmann wrote:
> On 7/24/22 20:25, Michael Straube wrote:
> > The function dump_chip_info() is only used in rtl8188e_hal_init.c.
> > Make it static to reduce the driver object file size by 281 bytes.
> > 
> > before:
> >    text    data     bss     dec     hex filename
> > 530606   43897    7072  581575   8dfc7 drivers/staging/r8188eu/r8188eu.o
> > 
> > after:
> >    text    data     bss     dec     hex filename
> > 530405   43817    7072  581294   8deae drivers/staging/r8188eu/r8188eu.o
[]
> > diff --git a/drivers/staging/r8188eu/hal/rtl8188e_hal_init.c b/drivers/staging/r8188eu/hal/rtl8188e_hal_init.c
[]
> > @@ -526,6 +526,45 @@ void rtl8188e_ReadEFuse(struct adapter *Adapter, u16 _size_byte, u8 *pbuf)
> >   	Hal_EfuseReadEFuse88E(Adapter, 0, _size_byte, pbuf);
> >   }
> >   
> > +static void dump_chip_info(struct HAL_VERSION chip_vers)
> > +{
> > +	uint cnt = 0;
> > +	char buf[128];
> > +
> > +	cnt += sprintf((buf + cnt), "Chip Version Info: CHIP_8188E_");
> > +	cnt += sprintf((buf + cnt), "%s_", IS_NORMAL_CHIP(chip_vers) ?
> > +		       "Normal_Chip" : "Test_Chip");
> > +	cnt += sprintf((buf + cnt), "%s_", IS_CHIP_VENDOR_TSMC(chip_vers) ?
> > +		       "TSMC" : "UMC");
> > +
> > +	switch (chip_vers.CUTVersion) {
> > +	case A_CUT_VERSION:
> > +		cnt += sprintf((buf + cnt), "A_CUT_");
> > +		break;
> > +	case B_CUT_VERSION:
> > +		cnt += sprintf((buf + cnt), "B_CUT_");
> > +		break;
> > +	case C_CUT_VERSION:
> > +		cnt += sprintf((buf + cnt), "C_CUT_");
> > +		break;
> > +	case D_CUT_VERSION:
> > +		cnt += sprintf((buf + cnt), "D_CUT_");
> > +		break;
> > +	case E_CUT_VERSION:
> > +		cnt += sprintf((buf + cnt), "E_CUT_");
> > +		break;
> > +	default:
> > +		cnt += sprintf((buf + cnt), "UNKNOWN_CUT(%d)_", chip_vers.CUTVersion);
> > +		break;
> > +	}
> > +
> > +	cnt += sprintf((buf + cnt), "1T1R_");
> > +
> > +	cnt += sprintf((buf + cnt), "RomVer(%d)\n", 0);
> > +
> > +	pr_info("%s", buf);
> > +}

Probably better and likely smaller without the temporary buffer
and the possibility of overrun.

Something like:

{
	char cut[25];
	char *v = NULL;

	switch (chip_vers.CUTVersion) {
	case A_CUT_VERSION:
		v = "A_CUT";
		break;
	case B_CUT_VERSION:
		v = "B_CUT";
		break;
	case C_CUT_VERSION:
		v = "C_CUT";
		break;
	case D_CUT_VERSION:
		v = "D_CUT";
		break;
	case E_CUT_VERSION:
		v = "E_CUT";
		break;
	default:
		snprintf(cut, sizeof(cut), "UNKNOWN_CUT(%d)", chip_vers.CUTVersion);
		v = cut;
		break;
	}
	
	pr_info("Chip Version Info: CHIP_8188E_%s__Chip_%s_%s_1T1R_RomVer(%d)\n",
		IS_NORMAL_CHIP(chip_vers) ? "Normal" : "Test",
		IS_CHIP_VENDOR_TSMC(chip_vers) ? "TSMC" : "UMC",
		v, 0);
}