[PATCH] dprintf implementation

[PATCH] dprintf implementation

[Vincent Pelletier]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hello.
I haven't worked on grub2 for 2 weeks now, sorry.
Here is the dprintf function I worked on 2 weeks ago, which I fixed this
evening.

Vincent Pelletier

2005-02-21  Vincent Pelletier  <subdino2004@yahoo.fr>

	* include/grub/misc.h (grub_dprintf): New macro.
	(grub_real_dprintf): New prototype.
	* kern/misc.c (grub_real_dprintf): New function.

Index: kern/misc.c
===================================================================
RCS file: /cvsroot/grub/grub2/kern/misc.c,v
retrieving revision 1.18
diff -u -p -r1.18 misc.c
- --- kern/misc.c	19 Feb 2005 20:56:07 -0000	1.18
+++ kern/misc.c	21 Feb 2005 20:38:38 -0000
@@ -128,6 +128,18 @@ grub_printf (const char *fmt, ...)
~   return ret;
~ }

+void
+grub_real_dprintf(const char *file, const int line, const char *title,
+                  const char *fmt, ...)
+{
+  va_list args;
+
+  grub_printf ("%s,%d (%s): ", file, line, title);
+  va_start (args, fmt);
+  grub_vprintf (fmt, args);
+  va_end (args);
+}
+
~ int
~ grub_vprintf (const char *fmt, va_list args)
~ {
Index: include/grub/misc.h
===================================================================
RCS file: /cvsroot/grub/grub2/include/grub/misc.h,v
retrieving revision 1.12
diff -u -p -r1.12 misc.h
- --- include/grub/misc.h	29 Jan 2005 22:01:53 -0000	1.12
+++ include/grub/misc.h	21 Feb 2005 20:38:38 -0000
@@ -26,6 +26,7 @@
~ #include <grub/symbol.h>
~ #include <grub/err.h>

+#define grub_dprintf(title,fmt,args...)
grub_real_dprintf(__FILE__,__LINE__,title,fmt,args);
~ /* XXX: If grub_memmove is too slow, we must implement grub_memcpy.  */
~ #define grub_memcpy(d,s,n)	grub_memmove ((d), (s), (n))

@@ -58,6 +59,10 @@ char *EXPORT_FUNC(grub_strndup) (const c
~ void *EXPORT_FUNC(grub_memset) (void *s, int c, grub_size_t n);
~ grub_size_t EXPORT_FUNC(grub_strlen) (const char *s);
~ int EXPORT_FUNC(grub_printf) (const char *fmt, ...) __attribute__
((format (printf, 1, 2)));
+void EXPORT_FUNC(grub_real_dprintf) (const char *file,
+                                     const int line,
+                                     const char *title,
+                                     const char *fmt, ...)
__attribute__ ((format (printf, 4, 5)));
~ int EXPORT_FUNC(grub_vprintf) (const char *fmt, va_list args);
~ int EXPORT_FUNC(grub_sprintf) (char *str, const char *fmt, ...)
__attribute__ ((format (printf, 2, 3)));
~ int EXPORT_FUNC(grub_vsprintf) (char *str, const char *fmt, va_list args);
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.0 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFCGkzqFEQoKRQyjtURAokmAJ9Mirp8xseh2AwUJOl+tykvKm/lHgCeM07L
2pNw3UbeFYot+LOR4vDFWH4=
=+3BC
-----END PGP SIGNATURE-----

Re: [PATCH] dprintf implementation

[Hollis Blanchard]

On Feb 21, 2005, at 3:04 PM, Vincent Pelletier wrote:
> Here is the dprintf function I worked on 2 weeks ago, which I fixed 
> this
> evening.

Your mailer did some bad stuff to this patch again...

> Index: kern/misc.c
> ===================================================================
> RCS file: /cvsroot/grub/grub2/kern/misc.c,v
> retrieving revision 1.18
> diff -u -p -r1.18 misc.c
> - --- kern/misc.c	19 Feb 2005 20:56:07 -0000	1.18
> +++ kern/misc.c	21 Feb 2005 20:38:38 -0000
> @@ -128,6 +128,18 @@ grub_printf (const char *fmt, ...)
> ~   return ret;
> ~ }
>
> +void
> +grub_real_dprintf(const char *file, const int line, const char *title,
> +                  const char *fmt, ...)
> +{
> +  va_list args;
> +
> +  grub_printf ("%s,%d (%s): ", file, line, title);
> +  va_start (args, fmt);
> +  grub_vprintf (fmt, args);
> +  va_end (args);
> +}
> +
> ~ int
> ~ grub_vprintf (const char *fmt, va_list args)
> ~ {

Weren't we going to have various flags to selectively enable debug 
messages from particular components at runtime? That's the most 
important thing to me: to be able to debug a user's mysterious problem 
by telling them "use debug=filesystem and send me the output".

-Hollis

Re: [PATCH] dprintf implementation

[Vincent Pelletier]

Hollis Blanchard wrote:
> Your mailer did some bad stuff to this patch again...

Argh. I wonder if it is because I sign my mails...

> Weren't we going to have various flags to selectively enable debug 
> messages from particular components at runtime?

Iirc there were differences of point of view on that matter.
Okuji, what do you think about that idea ?

What about something like :
GRUB> DEBUG=D_PARTMAP|D_FS
GRUB> DEBUG_OUTPUT=D_SCREEN
GRUB> command_to_be_watched
file.c,line (D_PARTMAP): Partmap of type 'foo' found.
file2.c,line (D_FS): Superblock valid, fs 'bar'.
[...]

I see those points to be done :
- define | operator
- name-to-value conversion for constants (case-sensitive ?)
- value-to-name conversion for constants

What about modules ? It should be good to make them able to add new
error constants (maybe only one per module ?) that would match their own
dprintf.

Here are some prototypes as I think of them:
char *grub_debug_constants[sizeof(grub_debug_mask)*8];
int grub_debug_constant_request (const char *name);
   return value : -1 = no more available bit, -2 = duplicate name,
   otherwise 1<<value would give the position of the bit in the mask.
   That binary shift would be done by grub_dprintf. Checks for duplicate
   name (again, case sensitive ?).
void grub_dprintf (int value, char *format, ...);
   "value" would be the one returned by grub_debug_constant_request.
void grub_debug_constant_release (int value);

Vincent Pelletier

Re: [PATCH] dprintf implementation

[Aki Tossavainen]

On Thu, 24 Feb 2005, Vincent Pelletier wrote:

> Hollis Blanchard wrote:
> > Your mailer did some bad stuff to this patch again...
>
> Argh. I wonder if it is because I sign my mails...
>
> > Weren't we going to have various flags to selectively enable debug
> > messages from particular components at runtime?
>
> Iirc there were differences of point of view on that matter.
> Okuji, what do you think about that idea ?
>
> What about something like :
> GRUB> DEBUG=D_PARTMAP|D_FS
> GRUB> DEBUG_OUTPUT=D_SCREEN
> GRUB> command_to_be_watched
> file.c,line (D_PARTMAP): Partmap of type 'foo' found.
> file2.c,line (D_FS): Superblock valid, fs 'bar'.
> [...]
>
> I see those points to be done :
> - define | operator
> - name-to-value conversion for constants (case-sensitive ?)
> - value-to-name conversion for constants
>
> What about modules ? It should be good to make them able to add new
> error constants (maybe only one per module ?) that would match their own
> dprintf.
>
> Here are some prototypes as I think of them:
> char *grub_debug_constants[sizeof(grub_debug_mask)*8];
> int grub_debug_constant_request (const char *name);
>    return value : -1 = no more available bit, -2 = duplicate name,
>    otherwise 1<<value would give the position of the bit in the mask.
>    That binary shift would be done by grub_dprintf. Checks for duplicate
>    name (again, case sensitive ?).
> void grub_dprintf (int value, char *format, ...);
>    "value" would be the one returned by grub_debug_constant_request.
> void grub_debug_constant_release (int value);
>
> Vincent Pelletier
>
>
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>

Signing emails does damage to dashes. It's because consecutive dashes are
treated as signature boundary mark. It changes ---- to - ---.

Aki Tossavainen

Re: [PATCH] dprintf implementation

[Yoshinori K. Okuji]

On Thursday 24 February 2005 09:02, Vincent Pelletier wrote:
> Iirc there were differences of point of view on that matter.
> Okuji, what do you think about that idea ?

What I said was that you should use strings instead of bit fields.

# This enables the debug mode for fs and disk.
debug="fs disk"

...execute commands...

# This disable the debug mode.
debug=""


Here you don't have to use bit fields (such as DEBUG_MODE_FS_BIT) at 
all. When dprintf is called, dprintf simply check if a specified 
category is included in the variable "debug". Like this:

  enabled = 0;
  if (grub_strstr (debug, "all"))
    enabled = 1;
  else if (grub_strstr (debug, category))
    enabled = 1;

Actually, strstr is not appropriate, because it does not consider word 
boundaries.

Strings are much better because of the flexibility.

Okuji

Re: [PATCH] dprintf implementation

[Vincent Pelletier]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Yoshinori K. Okuji wrote:
| Actually, strstr is not appropriate, because it does not consider word
| boundaries.

I think I'll write some function to find a word in a string... There is
an algorithm I would like to test.

| Strings are much better because of the flexibility.

I agree now that I exactly understand what you were talking about :).

I'm working on it.

Vincent Pelletier
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.0 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFCHlhCFEQoKRQyjtURAhcoAJ9JSnbFh4jon7SNRbPMs52WcpR6lwCcD5qJ
zDVkYGnPV6kL0DJR/4bg9FQ=
=BTYg
-----END PGP SIGNATURE-----

Re: [PATCH] dprintf implementation

[Hollis Blanchard]

On Feb 24, 2005, at 1:30 PM, Yoshinori K. Okuji wrote:

> On Thursday 24 February 2005 09:02, Vincent Pelletier wrote:
>> Iirc there were differences of point of view on that matter.
>> Okuji, what do you think about that idea ?
>
> What I said was that you should use strings instead of bit fields.
> [...]
> Here you don't have to use bit fields (such as DEBUG_MODE_FS_BIT) at
> all. When dprintf is called, dprintf simply check if a specified
> category is included in the variable "debug". Like this:

Ah ok, so the call would look like this:
	grub_dprintf("disk", "reading block %i", blockno);

Although that could lend itself to difficult-to-detect typos. Perhaps a 
#define would be useful after all:
	#define GRUB_DBG_DISK "disk"
	grub_dprintf(GRUB_DBG_DISK, "reading block %i", blockno);

Ah, I see Vincent's patch already does what's needed with the 
grub_dprintf macro, so it's just the conditional output he's working on 
in grub_real_dprintf. Sounds good. :)

-Hollis

Re: [PATCH] dprintf implementation

[Aki Tossavainen]

On Thu, 24 Feb 2005, Hollis Blanchard wrote:

> On Feb 24, 2005, at 1:30 PM, Yoshinori K. Okuji wrote:
>
> > On Thursday 24 February 2005 09:02, Vincent Pelletier wrote:
> >> Iirc there were differences of point of view on that matter.
> >> Okuji, what do you think about that idea ?
> >
> > What I said was that you should use strings instead of bit fields.
> > [...]
> > Here you don't have to use bit fields (such as DEBUG_MODE_FS_BIT) at
> > all. When dprintf is called, dprintf simply check if a specified
> > category is included in the variable "debug". Like this:
>
> Ah ok, so the call would look like this:
> 	grub_dprintf("disk", "reading block %i", blockno);
>
> Although that could lend itself to difficult-to-detect typos. Perhaps a
> #define would be useful after all:
> 	#define GRUB_DBG_DISK "disk"
> 	grub_dprintf(GRUB_DBG_DISK, "reading block %i", blockno);
>
> Ah, I see Vincent's patch already does what's needed with the
> grub_dprintf macro, so it's just the conditional output he's working on
> in grub_real_dprintf. Sounds good. :)
>
> -Hollis
>
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>

Why do you use strings instead of numbers? Use of bitmask would allow
pretty simple config file entry like debug=1231. Just a thought.

Aki Tossavainen

[PATCHv2] dprintf implementation

[Vincent Pelletier]

[-- Attachment #1: Type: text/plain, Size: 760 bytes --]

dprintf patch, take 2.

grub_strword (string, word) : searches for word (a serie of
non-word-separators eventualy ended by word-separators) in string (a
succession of 0 or more words which can begin by word-separator(s))
grub_iswordseparator : matches any separator

Note : var="value1 value2" currently stores
"value1
in var, so I would advice using ',' for tests.

I hope this time nothing will be wrong with mime & '~'.

Vincent Pelletier

2005-02-25  Vincent Pelletier  <subdino2004@yahoo.fr>

	* include/grub/misc.h (grub_dprintf): New macro.
	(grub_real_dprintf): New prototype.
	(grub_strword): Likewise.
	(grub_iswordseparator): Likewise.
	* kern/misc.c (grub_real_dprintf): New function.
	(grub_strword): Likewise.
	(grub_iswordseparator): Likewise.


[-- Attachment #2: dprintf_2.diff --]
[-- Type: audio/x-mp3, Size: 3812 bytes --]

Re: [PATCHv2] dprintf implementation

[Hollis Blanchard]

On Feb 25, 2005, at 5:52 AM, Vincent Pelletier wrote:
>
> grub_strword (string, word) : searches for word (a serie of
> non-word-separators eventualy ended by word-separators) in string (a
> succession of 0 or more words which can begin by word-separator(s))

grub_strword looks a little overcomplicated; would something like this 
work?

int
grub_strword (const char *haystack, const char *needle)
{
     int pos = 0;
     int found = 0;

     while (haystack[pos]) {
         /* Advance to next word. */
         while (grub_iswordseparator (haystack[pos]))
             pos++;

         if (0 == grub_strcmp (&haystack[pos], needle))
             {
                 found = 1;
                 break;
             }
     }

     return found;
}

That assumes 'needle' contains no separator characters. I think that's 
a safe assumption, given that 'needle' should come from a list of 
#defines. Or we could even sanity-check that in grub_strword, which 
IMHO would still be simpler than your earlier code.

Also, don't forget to add double spaces after periods in comments; 
otherwise the emacs^W"style" police will kick in your door and make you 
write 500 ChangeLog entries as punishment.

-Hollis

Re: [PATCHv2] dprintf implementation

[Aki Tossavainen]

On Fri, 25 Feb 2005, Hollis Blanchard wrote:

> On Feb 25, 2005, at 5:52 AM, Vincent Pelletier wrote:
> >
> > grub_strword (string, word) : searches for word (a serie of
> > non-word-separators eventualy ended by word-separators) in string (a
> > succession of 0 or more words which can begin by word-separator(s))
>
> grub_strword looks a little overcomplicated; would something like this
> work?
>
Um, well, it does a lot of unnecessary expensive strcmp's there...

But if you add this, you can at least reduce them abit, since you are only
interested on the words.

> int
> grub_strword (const char *haystack, const char *needle)
> {
>      int pos = 0;
>      int found = 0;
>
>      while (haystack[pos]) {
>          /* Advance to next word. */
>          while (grub_iswordseparator (haystack[pos]))
>              pos++;
>
>          if (0 == grub_strcmp (&haystack[pos], needle))
>              {
>                  found = 1;
>                  break;
>              }
else {
  while (!grub_iswordseparator (haystack[pos]))
   pos++;
}


>      }
>
>      return found;
> }
>
> That assumes 'needle' contains no separator characters. I think that's
> a safe assumption, given that 'needle' should come from a list of
> #defines. Or we could even sanity-check that in grub_strword, which
> IMHO would still be simpler than your earlier code.
>
> Also, don't forget to add double spaces after periods in comments;
> otherwise the emacs^W"style" police will kick in your door and make you
> write 500 ChangeLog entries as punishment.
>
> -Hollis
>
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>

Re: [PATCHv2] dprintf implementation

[Hollis Blanchard]

On Feb 25, 2005, at 10:13 AM, Aki Tossavainen wrote:
> Um, well, it does a lot of unnecessary expensive strcmp's there...
>
> But if you add this, you can at least reduce them abit, since you are 
> only
> interested on the words.

Oops, yes, thanks. :)

-Hollis

Re: [PATCHv2] dprintf implementation

[Yoshinori K. Okuji]

On Friday 25 February 2005 17:02, Hollis Blanchard wrote:
> int
> grub_strword (const char *haystack, const char *needle)
> {
>      int pos = 0;
>      int found = 0;
>
>      while (haystack[pos]) {
>          /* Advance to next word. */
>          while (grub_iswordseparator (haystack[pos]))
>              pos++;
>
>          if (0 == grub_strcmp (&haystack[pos], needle))
>              {
>                  found = 1;
>                  break;
>              }
>      }
>
>      return found;
> }

grub_strword ("filesystem", "file") returns 0 in your implementation.

If I write this function, I use grub_strstr and check if the previous 
character and the next character are boundaries.

Okuji

Re: [PATCHv2] dprintf implementation

[Vincent Pelletier]

Yoshinori K. Okuji wrote:
> On Friday 25 February 2005 17:02, Hollis Blanchard wrote:
>>int
>>grub_strword (const char *haystack, const char *needle)
>>{
>>     int pos = 0;
>>     int found = 0;
>>
>>     while (haystack[pos]) {
>>         /* Advance to next word. */
>>         while (grub_iswordseparator (haystack[pos]))
>>             pos++;
>>
>>         if (0 == grub_strcmp (&haystack[pos], needle))
>>             {
>>                 found = 1;
>>                 break;
>>             }
>>     }
>>
>>     return found;
>>}

What about grub_strword("all,foo","all") ?
Wouldn't strcmp tell that {'a','l','l',',',...,'\0'} and 
{'a','l','l','\0'} are different ?

> grub_strword ("filesystem", "file") returns 0 in your implementation.

Isn't it what should happen ?
If we use 2 similar words, one being a substring of the other, I don't 
think the substring should make the larger match too. Or we might add a 
hierarchy thing (filesystem/inode, filesystem/block, kern/alloc, 
kern/term/font/draw, ...) where "filesystem" would match all 
"filesystem/*" ad "filesystem/inode" would only match that one.

Hollis: mmh, I bet you'll say it's overcomplicated :). And I think 
you're right. The more I think about such "simple" debug func, the more 
I feel like I'll take over the world of debugging system... Well, time 
to find some more of those nice mushrooms ;). And a clean paper to write 
those 500 Changelog entries.

Vincent Pelletier

Re: [PATCHv2] dprintf implementation

[Yoshinori K. Okuji]

On Friday 25 February 2005 19:04, Vincent Pelletier wrote:
> > grub_strword ("filesystem", "file") returns 0 in your
> > implementation.
>
> Isn't it what should happen ?

s/returns 0/returns 1/;

Okuji

Re: [PATCHv2] dprintf implementation

[Hollis Blanchard]

On Feb 25, 2005, at 11:12 AM, Yoshinori K. Okuji wrote:
>
> grub_strword ("filesystem", "file") returns 0 in your implementation.
>
> If I write this function, I use grub_strstr and check if the previous
> character and the next character are boundaries.

This is a very good idea. The standalone testcase below works for me:

int
grub_isspace (int c)
{
     return (c == '\n' || c == '\r' || c == ' ' || c == '\t');
}

int
grub_iswordseparator (int c)
{
     return (grub_isspace (c) || c == ',' || c == ';' || c == '|' || c 
== '&');
}

int
grub_strword(const char *haystack, const char *needle)
{
     char *match;
     char *end;

     match = strstr (haystack, needle);

     if (match == NULL)
         return 0;
     if ((match > haystack) && (!grub_iswordseparator (match[-1])))
         return 0;

     end = match + strlen(needle)+1;
     if (*end && !grub_iswordseparator (*end))
         return 0;

     return 1;
}

int main(void)
{
     int i;
     char *list[][2] = {
         { "foo", "bar" },
         { "file", "file" },
         { "filesystem", "file" },
         { NULL, NULL},
     };

     for (i=0; list[i][0] != NULL; i++) {
         if (grub_strword (list[i][0], list[i][1]))
             printf ("%s/%s matched\n", list[i][0], list[i][1]);
         else
             printf ("%s/%s didn't match\n", list[i][0], list[i][1]);
     }
     return 0;
}

I would like to make sure Vincent's patch gets in soon, so that as I 
debug I can leave conditional debug printfs in the code where they're 
needed. I believe grub_strword was the only subject of conversation in 
his patch; are there any other comments? If so please speak up soon... 
:)

-Hollis

Re: [PATCHv2] dprintf implementation

[Vincent Pelletier]

Hollis Blanchard wrote:
> int
> grub_strword(const char *haystack, const char *needle)
> {
>     char *match;
>     char *end;
> 
>     match = strstr (haystack, needle);
> 
>     if (match == NULL)
>         return 0;
>     if ((match > haystack) && (!grub_iswordseparator (match[-1])))
>         return 0;
> 
>     end = match + strlen(needle)+1;
>     if (*end && !grub_iswordseparator (*end))
>         return 0;
> 
>     return 1;
> }

I find a little problem (sorry :) ) :
haystack = "filesystem file"
needle = "file"
won't match.
A loop might do the trick, but after a short try I don't see how.

Here is a new version of my strword. It should be easier to read.

int
grub_strword (const char *haystack, const char *needle)
{
  const char *n_pos = needle;

  while (grub_iswordseparator (*haystack))
    haystack++;

  while (*haystack)
    {
      /* Crawl both the needle and the haystack word we're on.  */
      while(*haystack && !grub_iswordseparator (*haystack) && *haystack
== *n_pos)
        {
          haystack++;
          n_pos++;
        }

      /* If we reached the end of both words at the same time, the word
      is found. If not, eat everything in the haystack that isn't the
      next word (or the end of string) and "reset" the needle.  */
      if ( (!*haystack || grub_iswordseparator (*haystack))
         && (!*n_pos || grub_iswordseparator (*n_pos)))
        return 1;
      else
        {
          n_pos = needle;
          while (*haystack && !grub_iswordseparator (*haystack))
            haystack++;
          while (grub_iswordseparator (*haystack))
            haystack++;
        }
    }

  return 0;
}

Vincent Pelletier

Re: [PATCHv2] dprintf implementation

[Vincent Pelletier]

[-- Attachment #1: Type: text/plain, Size: 415 bytes --]

Here is a clean diff with that new grub_strword version.

Vincent Pelletier

2005-04-14  Vincent Pelletier  <subdino2004@yahoo.fr>

    * include/grub/misc.h (grub_dprintf): New macro.
    (grub_real_dprintf): New prototype.
    (grub_strword): Likewise.
    (grub_iswordseparator): Likewise.
    * kern/misc.c (grub_real_dprintf): New function.
    (grub_strword): Likewise.
    (grub_iswordseparator): Likewise.


[-- Attachment #2: dprintf_3.diff --]
[-- Type: audio/x-mp3, Size: 4026 bytes --]

Re: [PATCHv2] dprintf implementation

[Hollis Blanchard]

[-- Attachment #1: Type: text/plain, Size: 755 bytes --]

On Apr 14, 2005, at 9:08 AM, Vincent Pelletier wrote:

> Here is a clean diff with that new grub_strword version.

Thanks Vincent! I've just checked this in (with a slight tweak: needed 
to add ## to the macro to allow passing no arguments).

Now what do we think of this patch? It adds output like this:

grub> set debug=loader
grub> linux (hda,0)/zImage.chrp
../loader/powerpc/ieee1275/linux.c,207 : Loading segment 0 at 0x400060, 
size 0xe1000
grub> boot
../loader/powerpc/ieee1275/linux.c,59 : entry point: 0x400060
../loader/powerpc/ieee1275/linux.c,60 : initrd at: 0xc0000000, size 0x0
../loader/powerpc/ieee1275/linux.c,62 : /chosen/bootargs:
../loader/powerpc/ieee1275/linux.c,63 : jumping to Linux...

Are we happy with that formatting?

-Hollis

[-- Attachment #2: dprintf-loader.txt --]
[-- Type: text/plain, Size: 1862 bytes --]

Index: loader/powerpc/ieee1275/linux.c
===================================================================
RCS file: /cvsroot/grub/grub2/loader/powerpc/ieee1275/linux.c,v
retrieving revision 1.6
diff -u -p -w -r1.6 linux.c
--- loader/powerpc/ieee1275/linux.c	14 Feb 2005 18:41:33 -0000	1.6
+++ loader/powerpc/ieee1275/linux.c	9 May 2005 02:03:46 -0000
@@ -56,6 +56,12 @@ grub_linux_boot (void)
   grub_ieee1275_set_property (chosen, "bootargs", linux_args,
 			      grub_strlen (linux_args) + 1, &actual);
   
+  grub_dprintf ("loader", "entry point: 0x%x\n", linux_addr);
+  grub_dprintf ("loader", "initrd at: 0x%x, size 0x%x\n", initrd_addr,
+		initrd_size);
+  grub_dprintf ("loader", "/chosen/bootargs: %s\n", linux_args);
+  grub_dprintf ("loader", "jumping to Linux...\n");
+
   /* Boot the kernel.  */
   linuxmain = (kernel_entry_t) linux_addr;
   linuxmain ((void *) initrd_addr, initrd_size, grub_ieee1275_entry_fn, 0, 0);
@@ -188,13 +194,18 @@ grub_rescue_cmd_linux (int argc, char *a
 
       if (phdr->p_type == PT_LOAD)
 	{
+	  void *segment_addr = ((char *) entry) + offset;
+
 	  if (grub_file_seek (file, phdr->p_offset) == -1)
 	    {
 	      grub_error (GRUB_ERR_BAD_OS, "Invalid offset in program header");
 	      goto fail;
 	    }
 	  
-	  if (grub_file_read (file, (void *) (((char *) entry) + offset) , phdr->p_filesz) 
+	  grub_dprintf ("loader", "Loading segment %d at %p, size 0x%x\n", i,
+			segment_addr, phdr->p_filesz);
+
+	  if (grub_file_read (file, segment_addr , phdr->p_filesz) 
 	      != (grub_ssize_t) phdr->p_filesz)
 	    goto fail;
 	  
@@ -275,6 +286,8 @@ grub_rescue_cmd_initrd (int argc, char *
       goto fail;
     }
   
+  grub_dprintf ("loader", "Loading initrd at 0x%x, size 0x%x\n", addr, size);
+
   if (grub_file_read (file, (void *) addr, size) != size)
     {
       grub_ieee1275_release (addr, size);

Re: [PATCHv2] dprintf implementation

[Yoshinori K. Okuji]

On Thursday 14 April 2005 05:31 am, Hollis Blanchard wrote:
> I would like to make sure Vincent's patch gets in soon, so that as I
> debug I can leave conditional debug printfs in the code where they're
> needed. I believe grub_strword was the only subject of conversation in
> his patch; are there any other comments? If so please speak up soon...

Please check it in, if it works for you.

Okuji