* Just a cosmetic question about grub_vprintf()?
@ 2009-11-22 13:10 rubisher
2009-11-23 14:44 ` Robert Millan
0 siblings, 1 reply; 6+ messages in thread
From: rubisher @ 2009-11-22 13:10 UTC (permalink / raw)
To: The development of GRUB 2
Hello all,
I am just reading kernel/misc.c file and read grub_vsprintf() definition as:
int
grub_vsprintf (char *str, const char *fmt, va_list args)
which is used by:
int
grub_vprintf (const char *fmt, va_list args)
{
int ret;
ret = grub_vsprintf (0, fmt, args);
return ret;
}
But as far as the 1st parameter of grub_vsprintf is a pointer,
wouldn't it be better to write:
--- kern/misc.c.orig 2009-11-22 13:07:22.000000000 +0000
+++ kern/misc.c 2009-11-22 13:07:51.000000000 +0000
@@ -160,7 +160,7 @@
{
int ret;
- ret = grub_vsprintf (0, fmt, args);
+ ret = grub_vsprintf (NULL, fmt, args);
return ret;
}
Tia,
J.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Just a cosmetic question about grub_vprintf()?
2009-11-22 13:10 Just a cosmetic question about grub_vprintf()? rubisher
@ 2009-11-23 14:44 ` Robert Millan
2009-11-28 18:21 ` rubisher
0 siblings, 1 reply; 6+ messages in thread
From: Robert Millan @ 2009-11-23 14:44 UTC (permalink / raw)
To: The development of GNU GRUB
On Sun, Nov 22, 2009 at 01:10:11PM +0000, rubisher wrote:
> But as far as the 1st parameter of grub_vsprintf is a pointer,
> wouldn't it be better to write:
> --- kern/misc.c.orig 2009-11-22 13:07:22.000000000 +0000
> +++ kern/misc.c 2009-11-22 13:07:51.000000000 +0000
> @@ -160,7 +160,7 @@
> {
> int ret;
>
> - ret = grub_vsprintf (0, fmt, args);
> + ret = grub_vsprintf (NULL, fmt, args);
> return ret;
> }
Yes. But we have many of those, so we don't go huntin' them. If you'd
like to help us, a patch that does this change in bulk would be welcome.
Thanks
--
Robert Millan
The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
how) you may access your data; but nobody's threatening your freedom: we
still allow you to remove your data and not access it at all."
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Just a cosmetic question about grub_vprintf()?
2009-11-23 14:44 ` Robert Millan
@ 2009-11-28 18:21 ` rubisher
2009-11-29 9:19 ` Colin Watson
0 siblings, 1 reply; 6+ messages in thread
From: rubisher @ 2009-11-28 18:21 UTC (permalink / raw)
To: The development of GNU GRUB
Robert Millan wrote:
> On Sun, Nov 22, 2009 at 01:10:11PM +0000, rubisher wrote:
>> But as far as the 1st parameter of grub_vsprintf is a pointer,
>> wouldn't it be better to write:
>> --- kern/misc.c.orig 2009-11-22 13:07:22.000000000 +0000
>> +++ kern/misc.c 2009-11-22 13:07:51.000000000 +0000
>> @@ -160,7 +160,7 @@
>> {
>> int ret;
>>
>> - ret = grub_vsprintf (0, fmt, args);
>> + ret = grub_vsprintf (NULL, fmt, args);
>> return ret;
>> }
>
> Yes. But we have many of those, so we don't go huntin' them. If you'd
> like to help us, a patch that does this change in bulk would be welcome.
>
> Thanks
>
It will be of great pleasure for me, but I didn't foreseen so much (the most difficult to me are 'opaque pointer') but I
hoppe that such 'sparse' would help me for the most ;<)
Hope to comeback soon,
J.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Just a cosmetic question about grub_vprintf()?
2009-11-28 18:21 ` rubisher
@ 2009-11-29 9:19 ` Colin Watson
2009-12-05 21:58 ` rubisher
0 siblings, 1 reply; 6+ messages in thread
From: Colin Watson @ 2009-11-29 9:19 UTC (permalink / raw)
To: The development of GNU GRUB
On Sat, Nov 28, 2009 at 06:21:20PM +0000, rubisher wrote:
> Robert Millan wrote:
>> On Sun, Nov 22, 2009 at 01:10:11PM +0000, rubisher wrote:
>>> But as far as the 1st parameter of grub_vsprintf is a pointer,
>>> wouldn't it be better to write:
>>> --- kern/misc.c.orig 2009-11-22 13:07:22.000000000 +0000
>>> +++ kern/misc.c 2009-11-22 13:07:51.000000000 +0000
>>> @@ -160,7 +160,7 @@
>>> {
>>> int ret;
>>>
>>> - ret = grub_vsprintf (0, fmt, args);
>>> + ret = grub_vsprintf (NULL, fmt, args);
>>> return ret;
>>> }
>>
>> Yes. But we have many of those, so we don't go huntin' them. If you'd
>> like to help us, a patch that does this change in bulk would be welcome.
>
> It will be of great pleasure for me, but I didn't foreseen so much (the
> most difficult to me are 'opaque pointer') but I hoppe that such 'sparse'
> would help me for the most ;<)
If you do this, make sure you understand why it makes no difference in
standards-compliant C. In particular, this understanding matters when
functions with variable-length argument lists are concerned.
(See the C FAQ for more details.)
--
Colin Watson [cjwatson@ubuntu.com]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Just a cosmetic question about grub_vprintf()?
2009-11-29 9:19 ` Colin Watson
@ 2009-12-05 21:58 ` rubisher
2009-12-09 21:36 ` Robert Millan
0 siblings, 1 reply; 6+ messages in thread
From: rubisher @ 2009-12-05 21:58 UTC (permalink / raw)
To: The development of GNU GRUB
Colin Watson wrote:
> On Sat, Nov 28, 2009 at 06:21:20PM +0000, rubisher wrote:
>> Robert Millan wrote:
>>> On Sun, Nov 22, 2009 at 01:10:11PM +0000, rubisher wrote:
>>>> But as far as the 1st parameter of grub_vsprintf is a pointer,
>>>> wouldn't it be better to write:
>>>> --- kern/misc.c.orig 2009-11-22 13:07:22.000000000 +0000
>>>> +++ kern/misc.c 2009-11-22 13:07:51.000000000 +0000
>>>> @@ -160,7 +160,7 @@
>>>> {
>>>> int ret;
>>>>
>>>> - ret = grub_vsprintf (0, fmt, args);
>>>> + ret = grub_vsprintf (NULL, fmt, args);
>>>> return ret;
>>>> }
>>> Yes. But we have many of those, so we don't go huntin' them. If you'd
>>> like to help us, a patch that does this change in bulk would be welcome.
>> It will be of great pleasure for me, but I didn't foreseen so much (the
>> most difficult to me are 'opaque pointer') but I hoppe that such 'sparse'
>> would help me for the most ;<)
>
> If you do this, make sure you understand why it makes no difference in
> standards-compliant C. In particular, this understanding matters when
> functions with variable-length argument lists are concerned.
>
> (See the C FAQ for more details.)
>
Ok, I will take care.
In the mean time I just have enough time to try this diff to add enable-sparse option using cgcc wrapper:
--- configure.ac 2009-12-05 10:12:22 +0000
+++ configure.ac 2009-12-05 19:14:14 +0000
@@ -537,6 +537,20 @@
[AC_DEFINE([MM_DEBUG], [1],
[Define to 1 if you enable memory manager debugging.])])
+AC_ARG_ENABLE([sparse],
+ AS_HELP_STRING([--enable-sparse],
+ [enable sparse code checking]), ,
+ enable_sparse=no
+)
+
+# Set cgcc as compiler and add sparse flags if --enable-sparse was specified.
+if test "$enable_sparse" = "yes"; then
+ CC="REAL_CC=$CC cgcc"
+ CFLAGS="$CFLAGS -Wbitwise -Wnon-pointer-null"
+ TARGET_CC="REAL_CC=$TARGET_CC cgcc"
+ TARGET_CFLAGS="$TARGET_CFLAGS -Wbitwise -Wnon-pointer-null"
+fi
+
AC_ARG_ENABLE([grub-emu-usb],
[AS_HELP_STRING([--enable-grub-emu-usb],
[build and install the `grub-emu' debugging utility with USB support (default=guessed)])])
=== <> ===
That seems to works on my side but all advise are welcome ;<)
Tx,
J.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Just a cosmetic question about grub_vprintf()?
2009-12-05 21:58 ` rubisher
@ 2009-12-09 21:36 ` Robert Millan
0 siblings, 0 replies; 6+ messages in thread
From: Robert Millan @ 2009-12-09 21:36 UTC (permalink / raw)
To: The development of GNU GRUB
On Sat, Dec 05, 2009 at 09:58:01PM +0000, rubisher wrote:
> +AC_ARG_ENABLE([sparse],
> + AS_HELP_STRING([--enable-sparse],
> + [enable sparse code checking]), ,
> + enable_sparse=no
> +)
> +
> +# Set cgcc as compiler and add sparse flags if --enable-sparse was specified.
> +if test "$enable_sparse" = "yes"; then
> + CC="REAL_CC=$CC cgcc"
> + CFLAGS="$CFLAGS -Wbitwise -Wnon-pointer-null"
> + TARGET_CC="REAL_CC=$TARGET_CC cgcc"
> + TARGET_CFLAGS="$TARGET_CFLAGS -Wbitwise -Wnon-pointer-null"
> +fi
> +
This doesn't seem to be recognized by all versions of GCC we support. If
we enable these flags this needs to be checked.
--
Robert Millan
The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
how) you may access your data; but nobody's threatening your freedom: we
still allow you to remove your data and not access it at all."
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-12-09 21:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-11-22 13:10 Just a cosmetic question about grub_vprintf()? rubisher
2009-11-23 14:44 ` Robert Millan
2009-11-28 18:21 ` rubisher
2009-11-29 9:19 ` Colin Watson
2009-12-05 21:58 ` rubisher
2009-12-09 21:36 ` Robert Millan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.