[PATCH] reentrant and .bss-less vsprintf

[Patrick Georgi]

Hi,

attached patch kills a global variable, and makes vsprintf reentrant and 
work without requiring .bss space.

Make vsprintf reentrant. More importantly, eliminate global variable.

Signed-off-by: Patrick Georgi 
Index: src/console/vsprintf.c
===================================================================
--- src/console/vsprintf.c	(Revision 1725)
+++ src/console/vsprintf.c	(Arbeitskopie)
@@ -15,20 +15,16 @@

-/* FIXME this global makes vsprintf non-reentrant */
-
-static char *str_buf;
-static void str_tx_byte(unsigned char byte)
-{
-	*str_buf = byte;
-	str_buf++;
-}
-
 int vsprintf(char * buf, const char *fmt, va_list args)
 {
+	char *str_buf;
+	void str_tx_byte(unsigned char byte)
+	{
+		*str_buf = byte;
+		str_buf++;
+	}
+
    int i;
    str_buf = buf;
    i = vtxprintf(str_tx_byte, fmt, args);
-	/* maeder/Ispiri -- The null termination was missing a deference */
-	/*                  and was just zeroing out the pointer instead */
    *str_buf = '\0';
    return i;
 }

[ron minnich]

Is it necessary to nest the function to fix this problem? Are you sure
no other function will ever need str_tx_byte?

ron

[Patrick Georgi]

ron minnich schrieb:
> Is it necessary to nest the function to fix this problem? Are you sure
> no other function will ever need str_tx_byte?

As far as I can see, the global variable is only used for communication 
between the two functions, of which one is a callback.
By nesting the function, the variable can be local.

The only alternative I see is to rewrite that part entirely.
Unless the gcc maintainers plan to kill this extension, I see no reason 
not to use it.

Patrick

[Stefan Reinauer]

On 26.05.2009 15:58 Uhr, Patrick Georgi wrote:
> ron minnich schrieb:
> > Is it necessary to nest the function to fix this problem? Are you sure
> > no other function will ever need str_tx_byte?
>
> As far as I can see, the global variable is only used for
> communication between the two functions, of which one is a callback.
> By nesting the function, the variable can be local.
>
> The only alternative I see is to rewrite that part entirely.
> Unless the gcc maintainers plan to kill this extension, I see no
> reason not to use it.

That said, I believe we shouldn't make it a fashion. Some projects use
nested functions for everything.
In this case, a nested function is the most beautiful way to fix our
problem, though.

I like Myles' idea to write a bit more of a comment a lot, though.

Stefan

[Carl-Daniel Hailfinger]

On 26.05.2009 15:58, Patrick Georgi wrote:
> ron minnich schrieb:
> > Is it necessary to nest the function to fix this problem? Are you sure
> > no other function will ever need str_tx_byte?
>
> As far as I can see, the global variable is only used for
> communication between the two functions, of which one is a callback.
> By nesting the function, the variable can be local.
>
> The only alternative I see is to rewrite that part entirely.

Dumb question: Can we simply copy the v3 code to v2 here? That would
also give us nice benefits like printk buffering in the long term.

Regards,
Carl-Daniel

[Stefan Reinauer]

On 26.05.2009 15:50 Uhr, ron minnich wrote:
> Is it necessary to nest the function to fix this problem? Are you sure
> no other function will ever need str_tx_byte?
>
> ron

As the code is right now, nothing could use str_tx_byte anyways, since
it's declared static ;-)

I don't think anything else would use it, as they would probably use
console_tx_byte()...

[Myles Watson]

It might be worth inserting a comment here documenting why you're
declaring this function inside vsprintf.
> int vsprintf(char * buf, const char *fmt, va_list args)
>  {
> char *str_buf;
> void str_tx_byte(unsigned char byte)

Acked-by: Myles Watson

Thanks,
Myles