linux-kernel - Re: [PATCH 3/3] V4L: stk11xx, add static to tables

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: <alpine.LFD.0.999.0708291448480.16082@enigma.security.iitk.ac.in>
Date:	Wed, 29 Aug 2007 16:12:39 +0530 (IST)
From:	Satyam Sharma <satyam@...radead.org>
To:	Jiri Slaby <jirislaby@...il.com>
cc:	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] V4L: stk11xx, add static to tables

Hi Jiri,

On Tue, 28 Aug 2007, Jiri Slaby wrote:
> 
> stk11xx, add static to tables
> 
> ensure, that the compiler will put all the tables in static storage

> @@ -26,7 +26,7 @@ static int stk1125_load_microcode(struct stk11xx *dev)
>  	int retok;
>  
>  	/* From 80x60 to 640x480 */
> -	const u8 values_1_204[] = {
> +	static const u8 values_1_204[] = {
>  		0x12, 0x11, 0x3b, 0x6a, 0x13, 0x10, 0x00, 0x01, 0x02, 0x13,
>  		0x39, 0x38, 0x37, 0x35, 0x0e, 0x12, 0x04, 0x0c, 0x0d, 0x17,
>  		0x18, 0x32, 0x19, 0x1a, 0x03, 0x1b, 0x16, 0x33, 0x34, 0x41,

Probably I'm missing something (or am wrong/mistaken, in which case I hope
somebody corrects me), but what do we get with this change?

I think having "static const" on data having function-local scope (as is
the case with all occurrences in this patch) is often redundant/pointless
thing to do. "static" on function-local data serves only to instruct the
compiler to allocate storage in .data instead of stack -- .data because it
lives for as long as the program will, and also because it is read-write.
"const", OTOH will send it to the .rodata section anyway -- which has
similar storage lifetime, with additional restriction that it is read-only
and also allows the compiler to do all sorts of optimization tricks
(because such data is known to remain constant during runtime as well).
For "static const", .rodata is chosen over .data, neither of which is the
function's stack, so effectively it's not any more beneficial than simple
"const", and if anything might only confuse the compiler.

Note that having "static const" on _global_ (not function-local) data is
still a useful thing to do -- "static" scopes it within the current file,
and "const" throws it into the .rodata with all its other implications.
But that is not the case with any of the cases in this particular patch.

The reason I ask is because I did something similar recently -- allocated
an array of constant strings -- const char *stringarray[xxx] = {...};
inside a function, and am even fearlessly returning a pointer to one of
those strings back to the caller(!) Things do work correctly as expected,
but is there something subtle I missed?

Satyam
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/