Re: [PATCH] Use getopt_long() instead of argp_parse() in grub-emu

[Christian Franke]

Marco Gerards wrote:

Christian Franke <[EMAIL PROTECTED]> writes:

  

Unlike the other GRUB2 utils, grub-emu uses the glibc extension
argp_parse(). It is unavailable on Cygwin, which might also be the
case for other platforms where glibc is not the native runtime.



This has been brought up before, BSD has the same problem.  The
outcome of the discussion was (IIRC) that we will use argp.  When argp
is not available we can use gnulib or a standalone argp parser
("argp-standalone") to support this.  In that case it will mean
changing configure.ac.

  


Will argp_parse() be a pre-requisite for building GRUB2 or will 
argp-standalone be included (some other projects do) ?


If you really want argp, why is it used only for the few trivial options 
of grub-emu ? The other utils still use getopt_long().
grub-emu does not benefit much from argp, but introduces another 
portability issue.


Christian



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

serial port

[Robert Millan]

Hi

Any reason why the serial port is not enabled in grub_serial_init() ?

grub> terminal
Available terminal(s): console
Current terminal: console
grub> serial
grub> terminal
Available terminal(s): serial console
Current terminal: serial

-- 
Robert Millan

 I know my rights; I want my phone call!
 What use is a phone call, if you are unable to speak?
(as seen on /.)


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Handle C symbols with leading underscore (HAVE_ASM_USCORE)

[Robert Millan]

On Sat, Nov 10, 2007 at 08:03:57PM +0100, Christian Franke wrote:
> 
> This patch fixes this by ignoring the difference. It actually works to 
> load modules compiled on Linux by a kernel compiled on Cygwin (with 
> underscores) and vice versa.
>
> [...]
> +
> +  /* Ignore leading underscore used for C symbols.
> + Done at runtime to allow loading modules compiled on other OS.  */
> +  if (*name == '_')
> +name++;

Is that something we really want?  We never made any efforts at maintaining
ABI between kernel and modules, or in allowing this kind of combinations.  If
we go this path, it can mean more work later (e.g. support for building kernel
and modules with different versions of GCC, etc).

Could you give an example of a situation in which this would be useful?

-- 
Robert Millan

 I know my rights; I want my phone call!
 What use is a phone call, if you are unable to speak?
(as seen on /.)


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Improving the website about GRUB 2 development

[Robert Millan]

On Sat, Nov 10, 2007 at 07:19:34PM +0100, Marco Gerards wrote:
> 
> If we put this on a website in a friendly and attractive way, we might
> get more contributions.

Getting more contributions sounds nice; but can we cope with them?  I
think it's more important to make sure we can properly (and timely!) attend
all the contributions we receive first.

-- 
Robert Millan

 I know my rights; I want my phone call!
 What use is a phone call, if you are unable to speak?
(as seen on /.)


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: GRUB 2 development

[Robert Millan]

On Sat, Nov 10, 2007 at 06:53:51PM +0100, Marco Gerards wrote:
> 
> The problem currently is that, accidently, we lose track of
> outstanding bugs and patches.  It is frustrating to both developers
> and people sending in patches/bugreports.
> 
> Hopefully we can start using a bug tracker soon.  There are many that
> are good.  In my opinion the mailinglist and wiki doesn't work for us
> anymore.
> 
> If we have a bug tracker, bugs and patches won't be forgotted until we
> actively close them.  This will fix a serious problem in GRUB
> development.  At the moment I am a bit more active.  But I can not
> promise if I can keep being this active...
> 
> Many projects use bugzilla.  Perhaps it isn't perfect, but it does
> what I want.  I just do not have the resources to set that up.
> Perhaps someone else knows something better.
> 
> I would even prefer using savannah than the current situation.  I hope
> we can figure out a solution real soon.  How about just using savannah
> and add a GRUB2 option there, unless someone comes up with something
> better in a few days/weeks?

We had this discussion before.  See:

  http://lists.gnu.org/archive/html/grub-devel/2007-06/msg00208.html

And:

  http://lists.gnu.org/archive/html/grub-devel/2007-07/msg00086.html
  (funny that we all forget things so easily ;-))

I think Okuji makes valid points.  How about using the tracker in savannah?

It's certainly better than using the mailing list as a bug tracker ;-)

-- 
Robert Millan

 I know my rights; I want my phone call!
 What use is a phone call, if you are unable to speak?
(as seen on /.)


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Status of ATA support

[Robert Millan]

On Sat, Nov 10, 2007 at 09:21:38PM +0200, Vesa Jääskeläinen wrote:
> Marco Gerards wrote:
> > First of all, this is mainly for i386-linuxbios.  On i386-pc we have
> > to disable biosdisk support because ata.mod and biosdisk.mod do not
> > like eachother :-).  Perhaps disk access via the BIOS will not be
> > possible/safe anymore after loading ata.mod.
> 
> We have to make sure we are not using BIOS for accessing disk after that
> point.

We already do (check for grub_disk_firmware_fini() function and
grub_disk_firmware_is_tainted variable).

But it has no effect on loaded OSes of course :-/

> And ATA controller should be at stable state after resuming
> normal operations OR we install custom int13h handler.

An int handler sounds like the sane thing to do..

-- 
Robert Millan

 I know my rights; I want my phone call!
 What use is a phone call, if you are unable to speak?
(as seen on /.)


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Avoid crash on empty menu

[Robert Millan]

On Sat, Nov 10, 2007 at 04:27:28PM +0100, Marco Gerards wrote:
> >
> > 2007-11-10  Christian Franke  <[EMAIL PROTECTED]>
> >
> > * normal/menu.c (run_menu): Check for empty menu to avoid crash.
> > (grub_menu_run): Likewise.
> 
> This looks ok to me.  We can apply this now, it's just a few line.

Committed.

-- 
Robert Millan

 I know my rights; I want my phone call!
 What use is a phone call, if you are unable to speak?
(as seen on /.)


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] fix serial console on LinuxBIOS

[Robert Millan]

On Sat, Nov 10, 2007 at 04:29:07PM +0100, Marco Gerards wrote:
> >> 
> >> No header ;)
> >
> > Uhm what header?
> 
> That's what I said ;-)
> 
> What I meant was something like:
> 
> 2007-10-31  Robert Millan  <[EMAIL PROTECTED]>

Ah, right.  I tend to skip this because the first part becomes
obsolete easily, the second part is the same for all commits (since
I don't change my name too often ;-)), and the last part too.

> What I meant was: what should and shouldn't be added to this .h?

I wouldn't add anything else.

> > autoconf already setups the cpu / machine symlinks.  Why ask it to tell the
> > same info twice?
> 
> Isn't that what you are doing now?  But I have no objections to this
> fix, please commit it :-)

Ok, done.

-- 
Robert Millan

 I know my rights; I want my phone call!
 What use is a phone call, if you are unable to speak?
(as seen on /.)


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] generic ELF version of grub-mkimage

[Robert Millan]

On Sat, Nov 10, 2007 at 06:29:12PM +0100, Marco Gerards wrote:
> Robert Millan <[EMAIL PROTECTED]> writes:
> 
> > On Fri, Oct 12, 2007 at 05:55:03PM +0200, Robert Millan wrote:
> >> 
> >> Ok.  But no, I didn't really notice.  I'll fix this entry and keep trying 
> >> to
> >> get it right next time.
> >
> > Does this look good?
> 
> This was committed, right?  Otherwise poke me and I will look :-)

No.

-- 
Robert Millan

 I know my rights; I want my phone call!
 What use is a phone call, if you are unable to speak?
(as seen on /.)


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: detecting other versions of SmartFirmware

[Robert Millan]

On Sat, Nov 10, 2007 at 06:46:28PM +0100, Marco Gerards wrote:
> Robert Millan <[EMAIL PROTECTED]> writes:
> 
> > 2007-09-05  Robert Millan  <[EMAIL PROTECTED]>
> >
> > * kern/powerpc/ieee1275/cmain.c (grub_ieee1275_test_flag): Detect
> > SmartFirmware version updates (as released by Sven Luther), and 
> > avoid
> > setting GRUB_IEEE1275_FLAG_NO_PARTITION_0 or
> > GRUB_IEEE1275_FLAG_0_BASED_PARTITIONS unless the running version is
> > known broken.
> 
> This doesn't break anything, right?  In that case we better have this
> applied soon :-)

It was already.

-- 
Robert Millan

 I know my rights; I want my phone call!
 What use is a phone call, if you are unable to speak?
(as seen on /.)


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] bug fix for the ntfs driver

[Robert Millan]

On Sat, Nov 10, 2007 at 06:31:09PM +0100, Marco Gerards wrote:
> 
> Sorry for forgetting about this patch.  It should be committed.
> 
> Robert, if you have time, can you commit this?  I will mark this mail
> as important... If you don't have the time, I will surely commit it
> ASAP :-)

Done.

-- 
Robert Millan

 I know my rights; I want my phone call!
 What use is a phone call, if you are unable to speak?
(as seen on /.)


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Fix eisa_mmap evaluation, add memory existence check

[Christian Franke]

Marco Gerards wrote:

Christian Franke <[EMAIL PROTECTED]> writes:
  

...
volatile is necessary here to tell the complier that the memory
address might not behave like regular memory. Otherwise, the optimizer
might legitimately remove memory accesses and then constant
propagation detects an unchanged value.

gcc actually does a very good job here. Result with volatile removed:



I think this is just normal memory, not memory mapped hardware.  Or
perhaps I am misunderstanding something here.

  


It is normal memory - but only if present :-) Like memory mapped 
hardware, missing memory violates the optimizers assumptions about 
memory accesses. Therefore, volatile is mandatory here.




$ gcc -S -O -fomit-frame-pointer init.c && cat init.s
...
addr_is_valid:
   movl$1, %eax
   ret
...


aka:

static int
addr_is_valid (grub_addr_t addr)
{
 return 1;
}


This is at least a proof that the original function returns the
correct result when real memory is present :-)



:-)

  

BTW: this also proves that the routine leaves memory unchanged.

Christian



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Standalone problem to test syntax rules

[Gregg Levine]

On Nov 10, 2007 1:43 PM, Marco Gerards <[EMAIL PROTECTED]> wrote:
> Bean <[EMAIL PROTECTED]> writes:
>
> >> In the tarball?
> >
> > Yes.
>
> Could you send the files to the list as attachments?  Or whatever is
> relevant.  tarballs on a mailinglist are very inconvenient. :-/
>
> --
> Marco
>
>
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>

Hello!
I agree! Those things on a list are driving people that way on the
LinuxBIOS list. While I won't name names, I can tell that the
responsible party is leaning towards conventional patches.

There is as it happens a number of places where a blob can be stored
for access by us, should you not be able to host them locally. Of
course I can't recall them, but certainly someone on this list can do
that.
-- 
Gregg C Levine [EMAIL PROTECTED]
"This signature was once found posting rude
 messages in English in the Moscow subway."


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: GRUB 2 development

[Jordi Mallach]

On Sat, Nov 10, 2007 at 06:53:51PM +0100, Marco Gerards wrote:
> Hopefully we can start using a bug tracker soon.  There are many that
> are good.  In my opinion the mailinglist and wiki doesn't work for us
> anymore.
> 
> If we have a bug tracker, bugs and patches won't be forgotted until we
> actively close them.  This will fix a serious problem in GRUB
> development.  At the moment I am a bit more active.  But I can not
> promise if I can keep being this active...
> 
> Many projects use bugzilla.  Perhaps it isn't perfect, but it does
> what I want.  I just do not have the resources to set that up.
> Perhaps someone else knows something better.

Have you considered using Trac? It's way lighter than bugzilla, and
offers a few other simple features that could ease this project's
management. Finding a place to host trac.enbug.org or whatever domain
would be trivial.

http://trac.edgewall.org/

Jordi
-- 
Jordi Mallach Pérez  --  Debian developer http://www.debian.org/
[EMAIL PROTECTED] [EMAIL PROTECTED] http://www.sindominio.net/
GnuPG public key information available at http://oskuro.net/


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Status of ATA support

[Vesa Jääskeläinen]

Marco Gerards wrote:
> First of all, this is mainly for i386-linuxbios.  On i386-pc we have
> to disable biosdisk support because ata.mod and biosdisk.mod do not
> like eachother :-).  Perhaps disk access via the BIOS will not be
> possible/safe anymore after loading ata.mod.

We have to make sure we are not using BIOS for accessing disk after that
point. And ATA controller should be at stable state after resuming
normal operations OR we install custom int13h handler. Actually you
could try to determine what devices are handled by biosdisk and what are
not. EDD supports some queries that report on what controller disk comes
from.



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: GRUB 2 development

[Vesa Jääskeläinen]

Marco Gerards wrote:
> Hi,
> 
> As you all might have noticed, I have been spamming this list like
> crazy the last two days :-)
> 
> The problem currently is that, accidently, we lose track of
> outstanding bugs and patches.  It is frustrating to both developers
> and people sending in patches/bugreports.
> 
> Hopefully we can start using a bug tracker soon.  There are many that
> are good.  In my opinion the mailinglist and wiki doesn't work for us
> anymore.
> 
> If we have a bug tracker, bugs and patches won't be forgotted until we
> actively close them.  This will fix a serious problem in GRUB
> development.  At the moment I am a bit more active.  But I can not
> promise if I can keep being this active...
> 
> Many projects use bugzilla.  Perhaps it isn't perfect, but it does
> what I want.  I just do not have the resources to set that up.
> Perhaps someone else knows something better.
> 
> I would even prefer using savannah than the current situation.  I hope
> we can figure out a solution real soon.  How about just using savannah
> and add a GRUB2 option there, unless someone comes up with something
> better in a few days/weeks?

I agree on this one. This has been a problem for a long time. And I
think we have had several discussions about this, so far no solution
however.

Problem with savannah is that it really doesn't handle different
releases nicely. And it does not handle patches nicely at all. Bugzilla
is much better on this approach. Though it is huge software to maintain
and needs a server. If we can find maintainer and the server with
necessary bandwidth (which should be small-to-moderate).

Oh... did I mention that bugzilla can be nicely combined with Mylyn :) ?
http://www.eclipse.org/mylyn/



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Improving the website about GRUB 2 development

[Vesa Jääskeläinen]

Marco Gerards wrote:
> Hi,
> 
> On the website there is no information about how to send in patches
> (so improving in the title is a bit of an understatement ;)).  I think
> we should add a page with information about patches that are sent in.
> That saves us some times to comment on obvious problems with a patch.
> But more importantly, this will save much time for the developer
> sending in the patch.
> 
> There are several things I would like to mention on this page:
> 
> 1) GCS/Changelogs/Coding Style
> 
> We can even duplicate some things from the GCS that are the most
> important.  I can't blame anyone for sending in a patch that doesn't
> match our coding style.  It isn't obvious from the website.  On the
> other hand, we have a coding style and I think we should use it.

Perhaps add a link to GCS and then pinpoint most important rules with
examples.

> 2) Copyrights
> 
> We might want to mention that we ask people to assign their copyrights
> or sign a disclaimer.  If people know it will be asked, it won't scare
> them away.  Or if it is a serious problem for them, people won't waste
> time writing a patch that can't be included.  I am willing to put my
> address there as a contact person in case people would like to ask
> questions about this privately.
> 
> We should also write something about code from 3rd parties and about
> looking at code from other parties and thus cause copyright issues.

It should also describe why. When reason is known it is much easier to
accept that.

> 3) How to send in patches.
> 
> At the moment patches will be sent to the mailinglist.  We have to
> mention we like a Changelog and an inlined patch, as attachment is
> ok.  No generated files, big patches are ok.
> 
> 4) Reviewing
> 
> Reality is that we do not have the manpower to review the patch in a
> few days.  If we mention this, it won't come as a surprise.

Perhaps patch processing process should be documented and as a note on
reviewing phase note about this.

> 5) Focus
> 
> Url to the todo list, bug list and an url to a site which states the
> priorities for the next release.

Ah... and those should then be also up-to-date :). I think we had
discussion some time about TODO file. These files should contain a note
to URL to wiki.

> If we put this on a website in a friendly and attractive way, we might
> get more contributions.  I think the current lack of information
> causes problems to us.  What do you guys think?  Did I miss something,
> or am I over enthusiastic?

I think it would be beneficial to have page that describes how to report
bugs and how to send patches. Especially the bug reporting page should
be targeting people that are not familiar on how to report bugs.


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] Handle C symbols with leading underscore (HAVE_ASM_USCORE)

[Christian Franke]

HAVE_ASM_USCORE is checked by configure, but module loader does not work 
in this case.


This patch fixes this by ignoring the difference. It actually works to 
load modules compiled on Linux by a kernel compiled on Cygwin (with 
underscores) and vice versa.


It also prepares the grub_mod_init/finit search for modules converted 
from non-ELF format (which do not support the "function" symbol type).


A bug in symbol table scan is fixed.

Christian

2007-11-10  Christian Franke  <[EMAIL PROTECTED]>

* genkernsyms.sh.in: Handle HAVE_ASM_USCORE case.

* genmk.rb: Handle HAVE_ASM_USCORE case in strip command.
Add $EXEEXT to CLEANFILES.

* kern/dl.c (grub_dl_resolve_symbol): Handle HAVE_ASM_USCORE case,
ignore leading underscore.
(grub_dl_register_symbol): Likewise.
(grub_dl_resolve_symbols): Likewise.
	Add check for grub_mod_init/fini for symbols without type. 
	(grub_dl_resolve_dependencies): Add check for trailing nullbytes

in symbol table. This fixes an infinite loop if stab is zero filled.


diff -rup grub2.orig/genkernsyms.sh.in grub2/genkernsyms.sh.in
--- grub2.orig/genkernsyms.sh.in	2006-07-12 22:42:52.0 +0200
+++ grub2/genkernsyms.sh.in	2007-10-06 12:59:29.0 +0200
@@ -16,9 +16,12 @@
 [EMAIL PROTECTED]@
 CC="@CC@"
 
+u=
+grep "^#define HAVE_ASM_USCORE" config.h >/dev/null 2>&1 && u="_"
+
 $CC -DGRUB_SYMBOL_GENERATOR=1 -E -I. -Iinclude -I$srcdir/include $* \
   | grep -v '^#' \
   | sed -n \
--e '/EXPORT_FUNC *([a-zA-Z0-9_]*)/{s/.*EXPORT_FUNC *(\([a-zA-Z0-9_]*\)).*/\1 kernel/;p;}' \
--e '/EXPORT_VAR *([a-zA-Z0-9_]*)/{s/.*EXPORT_VAR *(\([a-zA-Z0-9_]*\)).*/\1 kernel/;p;}' \
+-e '/EXPORT_FUNC *([a-zA-Z0-9_]*)/{s/.*EXPORT_FUNC *(\([a-zA-Z0-9_]*\)).*/'"$u"'\1 kernel/;p;}' \
+-e '/EXPORT_VAR *([a-zA-Z0-9_]*)/{s/.*EXPORT_VAR *(\([a-zA-Z0-9_]*\)).*/'"$u"'\1 kernel/;p;}' \
   | sort -u
diff -rup grub2.orig/genmk.rb grub2/genmk.rb
--- grub2.orig/genmk.rb	2007-10-20 22:49:38.96875 +0200
+++ grub2/genmk.rb	2007-11-10 19:37:17.56250 +0100
@@ -115,7 +115,7 @@ UNDSYMFILES += #{undsym}
 [EMAIL PROTECTED]: #{pre_obj} #{mod_obj}
 	-rm -f $@
 	$(TARGET_CC) $(#{prefix}_LDFLAGS) $(TARGET_LDFLAGS) -Wl,-r,-d -o $@ $^
-	$(STRIP) --strip-unneeded -K grub_mod_init -K grub_mod_fini -R .note -R .comment $@
+	$(STRIP) --strip-unneeded -K grub_mod_init -K grub_mod_fini -K _grub_mod_init -K _grub_mod_fini -R .note -R .comment $@
 
 #{pre_obj}: $(#{prefix}_DEPENDENCIES) #{objs_str}
 	-rm -f $@
@@ -187,7 +187,7 @@ class Utility
 deps = objs.collect {|obj| obj.suffix('d')}
 deps_str = deps.join(' ');
 
-"CLEANFILES += [EMAIL PROTECTED] #{objs_str}
+"CLEANFILES += [EMAIL PROTECTED](EXEEXT) #{objs_str}
 MOSTLYCLEANFILES += #{deps_str}
 
 [EMAIL PROTECTED]: $(#{prefix}_DEPENDENCIES) #{objs_str}
diff -rup grub2.orig/kern/dl.c grub2/kern/dl.c
--- grub2.orig/kern/dl.c	2007-07-22 01:32:26.0 +0200
+++ grub2/kern/dl.c	2007-11-10 19:44:34.140625000 +0100
@@ -156,6 +156,9 @@ grub_dl_resolve_symbol (const char *name
 {
   grub_symbol_t sym;
 
+  if (*name == '_')
+name++;
+
   for (sym = grub_symtab[grub_symbol_hash (name)]; sym; sym = sym->next)
 if (grub_strcmp (sym->name, name) == 0)
   return sym->addr;
@@ -169,6 +172,11 @@ grub_dl_register_symbol (const char *nam
 {
   grub_symbol_t sym;
   unsigned k;
+
+  /* Ignore leading underscore used for C symbols.
+ Done at runtime to allow loading modules compiled on other OS.  */
+  if (*name == '_')
+name++;
   
   sym = (grub_symbol_t) grub_malloc (sizeof (*sym));
   if (! sym)
@@ -347,6 +355,7 @@ grub_dl_resolve_symbols (grub_dl_t mod, 
   unsigned char type = ELF_ST_TYPE (sym->st_info);
   unsigned char bind = ELF_ST_BIND (sym->st_info);
   const char *name = str + sym->st_name;
+  int check_mod_func = 0;
   
   switch (type)
 	{
@@ -359,6 +368,12 @@ grub_dl_resolve_symbols (grub_dl_t mod, 
 		return grub_error (GRUB_ERR_BAD_MODULE,
    "the symbol `%s' not found", name);
 	}
+	  else if (sym->st_name != 0 && bind == STB_LOCAL)
+	{ /* Static functions have no type if initial format was not ELF.  */
+	  sym->st_value += (Elf_Addr) grub_dl_get_section_addr (mod,
+sym->st_shndx);
+	  check_mod_func = 1;
+	}
 	  else
 	sym->st_value = 0;
 	  break;
@@ -374,14 +389,11 @@ grub_dl_resolve_symbols (grub_dl_t mod, 
 	case STT_FUNC:
 	  sym->st_value += (Elf_Addr) grub_dl_get_section_addr (mod,
 sym->st_shndx);
-	  if (bind != STB_LOCAL)
+	  if (bind == STB_LOCAL)
+	check_mod_func = 1;
+	  else	
 	if (grub_dl_register_symbol (name, (void *) sym->st_value, mod))
 	  return grub_errno;
-	  
-	  if (grub_strcmp (name, "grub_mod_init") == 0)
-	mod->init = (void (*) (grub_dl_t)) sym->st_value;
-	  else if (grub_strcmp (name, "grub_mod_fini") == 0)
-	mod->fini = (void (*) (void)) sym->st_value;
 	  break;
 
 	case STT_SECTION:
@@ -397,6 +409,15 @@ grub_dl_res

Re: [PATCH] Building on Ubuntu 7.10 x86-64

[Vesa Jääskeläinen]

Marco Gerards wrote:
> Vesa Jääskeläinen <[EMAIL PROTECTED]> writes:
> 
> Hi Vesa,
> 
>> I needed to do following modification in order to build grub2 on my
>> 64bit laptop. In case there is no complaints I will commit it to CVS.
> 
> This looks ok to me.

Committed. New configure was also generated, so please report how well
it works.


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [patch] Fix for vga terminal

[Vesa Jääskeläinen]

Marco Gerards wrote:
> Vesa Jääskeläinen <[EMAIL PROTECTED]> writes:
> 
> Hi Vesa,
> 
>> Here is patch for fixing vga terminal. At least it worked fine on qemu.
>> Now it uses only font system to get fonts so you need to load font
>> module and use fonts. See gfxterm section on the wiki on how to do that.
>>
>> I had to edit patch by hand as 64bit patch is still pending and I need
>> that to compile my codes. But it should still apply nicely to current CVS.
> 
> This looks good to me.  Feel free to commit this! :-)

Done.


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Layout testing for graphical menu

[Vesa Jääskeläinen]

Marco Gerards wrote:
> Vesa Jääskeläinen <[EMAIL PROTECTED]> writes:
> 
> Hi,
> 
>> I was hoping that someone else might stand-up but as there wasn't really
>> interest I made a draft. This is basically using Okuji's idea of using
>> CSS for specifying look. I made HTML page and example CSS filling
>> information. And of course it looks ugly as I didn't spent too much on
>> tuning it.
> 
> Actually, I am still hoping you are interested in working on this! :-)
> 
> What is the state of the current framebuffer support?  Are
> backgrounds, etc supported?

I think I have a local copy that supports backgrounds. I was kinda
waiting that new menu system would take over this. But I think new plan
would be to do several iterations on it... so I might commit improved
gfxterm that supports background (and is a bit faster, though needs more
memory). Next iteration would be to replace this functionality with
improved menu/themes. That version probably will not be backwards
compatible with first iteration, but I think we can live with that.


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Standalone problem to test syntax rules

[Marco Gerards]

Bean <[EMAIL PROTECTED]> writes:

>> In the tarball? 
>
> Yes.

Could you send the files to the list as attachments?  Or whatever is
relevant.  tarballs on a mailinglist are very inconvenient. :-/

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Layout testing for graphical menu

[Marco Gerards]

Vesa Jääskeläinen <[EMAIL PROTECTED]> writes:

Hi,

> I was hoping that someone else might stand-up but as there wasn't really
> interest I made a draft. This is basically using Okuji's idea of using
> CSS for specifying look. I made HTML page and example CSS filling
> information. And of course it looks ugly as I didn't spent too much on
> tuning it.

Actually, I am still hoping you are interested in working on this! :-)

What is the state of the current framebuffer support?  Are
backgrounds, etc supported?

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: pxebooting with GRUB 2

[Daniel Kahn Gillmor]

On Sat 2007-11-10 12:25:10 -0500, Marco Gerards wrote:

> It will be committed soon, now for real ;-)

Thanks for the followup.  i await with bated breath; netboot is an
important feature for me!

Let me know if there's a patchset i can test at some point as an end
user.  Sorry i don't have the skills/time/knowledge to contribute
code in this case.

Regards,

--dkg


pgpJZzsL4F9IS.pgp
Description: PGP signature
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [patch] Fix for vga terminal

[Marco Gerards]

Vesa Jääskeläinen <[EMAIL PROTECTED]> writes:

Hi Vesa,

> Here is patch for fixing vga terminal. At least it worked fine on qemu.
> Now it uses only font system to get fonts so you need to load font
> module and use fonts. See gfxterm section on the wiki on how to do that.
>
> I had to edit patch by hand as 64bit patch is still pending and I need
> that to compile my codes. But it should still apply nicely to current CVS.

Great!  Thanks for fixing this!

[...]

> +2007-11-10  Vesa Jaaskelainen  <[EMAIL PROTECTED]>
> +
> + * conf/i386-pc.rmk (pkgdata_MODULES): Added vga.mod.
> + (vga_mod_SOURCES): Added.
> + (vga_mod_CFLAGS): Likewise.
> + (vga_mod_LDFLAGS): Likewise.
> +
> + * term/i386/pc/vga.c (get_map_mask): Switch order of arguments in
> + grub_outb() calls.
> + (set_map_mask): Likewise.
> + (set_read_map): Likewise.
> + (set_read_address): Likewise.
> + (vga_font): Removed variable.
> + (get_vga_glyph): Removed function.
> + (invalidate_char): Likewise.
> + (write_char): Changed to use grub_font_get_glyph() for font
> + information.
> + (grub_vga_putchar): Likewise.
> + (grub_vga_getcharwidth): Likewise.

This looks good to me.  Feel free to commit this! :-)

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Building on Ubuntu 7.10 x86-64

[Marco Gerards]

Vesa Jääskeläinen <[EMAIL PROTECTED]> writes:

Hi Vesa,

> I needed to do following modification in order to build grub2 on my
> 64bit laptop. In case there is no complaints I will commit it to CVS.

This looks ok to me.

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Improving the website about GRUB 2 development

[Marco Gerards]

Hi,

On the website there is no information about how to send in patches
(so improving in the title is a bit of an understatement ;)).  I think
we should add a page with information about patches that are sent in.
That saves us some times to comment on obvious problems with a patch.
But more importantly, this will save much time for the developer
sending in the patch.

There are several things I would like to mention on this page:

1) GCS/Changelogs/Coding Style

We can even duplicate some things from the GCS that are the most
important.  I can't blame anyone for sending in a patch that doesn't
match our coding style.  It isn't obvious from the website.  On the
other hand, we have a coding style and I think we should use it.

2) Copyrights

We might want to mention that we ask people to assign their copyrights
or sign a disclaimer.  If people know it will be asked, it won't scare
them away.  Or if it is a serious problem for them, people won't waste
time writing a patch that can't be included.  I am willing to put my
address there as a contact person in case people would like to ask
questions about this privately.

We should also write something about code from 3rd parties and about
looking at code from other parties and thus cause copyright issues.

3) How to send in patches.

At the moment patches will be sent to the mailinglist.  We have to
mention we like a Changelog and an inlined patch, as attachment is
ok.  No generated files, big patches are ok.

4) Reviewing

Reality is that we do not have the manpower to review the patch in a
few days.  If we mention this, it won't come as a surprise.

5) Focus

Url to the todo list, bug list and an url to a site which states the
priorities for the next release.

If we put this on a website in a friendly and attractive way, we might
get more contributions.  I think the current lack of information
causes problems to us.  What do you guys think?  Did I miss something,
or am I over enthusiastic?

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Status of ATA support

[Marco Gerards]

Hi,

A while ago I added support for ATA support.  It wasn't complete yet,
but I rather have this in CVS than rotting on my harddisk.  Besides
that, it isn't *that* bad.  I will describe the outstanding issues
below.

First of all, this is mainly for i386-linuxbios.  On i386-pc we have
to disable biosdisk support because ata.mod and biosdisk.mod do not
like eachother :-).  Perhaps disk access via the BIOS will not be
possible/safe anymore after loading ata.mod.

PCI devices are not supported *yet*.  Same for controllers 3 and 4.  I
will work on this.

The code to detect the type of controller (PATA, SATA, PATAPI, SATAPI)
really sucks.  I will have a look at this.

There is no code to detect the amount of sectors for a CDROM.  I will
write this eventually.  Patches are more than welcome!

The biggest problem is that grub_get_rtc is used to measure how long
we have to wait for the hardware to settle.  This function return the
amount of ticks (1/18th of a second), this resolution is not
acceptable.  It will result in large delays in ata.mod because it will
wait at least 1/18th of a second on i386-pc.

For i386-linuxbios we still need grub_get_rtc.

More about this on my blog:
http://www.mgerards.net/blog/?p=34

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Allow build without dirent.d_type, fix "ls (host)" crash

[Christian Franke]

Marco Gerards wrote:

...
2007-11-09  Christian Franke  <[EMAIL PROTECTED]>

* util/hostfs.c (is_dir): New function.
(grub_hostfs_dir):  Handle missing dirent.d_type case.
(grub_hostfs_read): Add missing fseek().
(grub_hostfs_label): Clear label pointer.  This fixes a crash
of grub-emu on "ls (host)".



Looks fine to me.
  


Thanks.


btw, when do you use (host)?  It's intended as a debugging tool.
  


It was there, it didn't work, and it breaked remaining grub-emu device 
access at all. So I fixed it :-)


Christian



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

GRUB 2 development

[Marco Gerards]

Hi,

As you all might have noticed, I have been spamming this list like
crazy the last two days :-)

The problem currently is that, accidently, we lose track of
outstanding bugs and patches.  It is frustrating to both developers
and people sending in patches/bugreports.

Hopefully we can start using a bug tracker soon.  There are many that
are good.  In my opinion the mailinglist and wiki doesn't work for us
anymore.

If we have a bug tracker, bugs and patches won't be forgotted until we
actively close them.  This will fix a serious problem in GRUB
development.  At the moment I am a bit more active.  But I can not
promise if I can keep being this active...

Many projects use bugzilla.  Perhaps it isn't perfect, but it does
what I want.  I just do not have the resources to set that up.
Perhaps someone else knows something better.

I would even prefer using savannah than the current situation.  I hope
we can figure out a solution real soon.  How about just using savannah
and add a GRUB2 option there, unless someone comes up with something
better in a few days/weeks?

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: detecting other versions of SmartFirmware

[Marco Gerards]

Robert Millan <[EMAIL PROTECTED]> writes:

> 2007-09-05  Robert Millan  <[EMAIL PROTECTED]>
>
> * kern/powerpc/ieee1275/cmain.c (grub_ieee1275_test_flag): Detect
> SmartFirmware version updates (as released by Sven Luther), and avoid
> setting GRUB_IEEE1275_FLAG_NO_PARTITION_0 or
> GRUB_IEEE1275_FLAG_0_BASED_PARTITIONS unless the running version is
> known broken.

This doesn't break anything, right?  In that case we better have this
applied soon :-)

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Using GRUB2 for booting from CD

[Marco Gerards]

"Alex Roman" <[EMAIL PROTECTED]> writes:

Hi Alex,

> On 20/08/07, Robert Millan <[EMAIL PROTECTED]> wrote:
>> I heard from Patrick Georgi on IRC (CCed) that he already has an ATAPI
>> implementation for GRUB as part of his LinuxBIOS work.  Maybe you should
>> coordinate with him so that the code can be shared with pc/bios targets.
>
> Awesome, perhaps we'll be able to get it going after Summer of Code is
> finished and all is integrated in the CVS, or if Patrick can post a
> patch that'd work too.

Did you send in a patch+Changelog entry with your work?

I would be happy to review it so we can integrate your work soon.

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: grub2 ChangeLog conf/i386-pc.rmk conf/powerpc-i...

[Marco Gerards]

"Jerone Young" <[EMAIL PROTECTED]> writes:

> That sounds reasonable. So perhaps having a multiboot directory as
> Marco specified is the best way to go then. Let me know if this is an
> a way everyone agrees on and I'll get a patch together to make things
> nice for everyone.

I hear no objections... :-)

Too bad CVS sucks for these kinds of things :-)

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: New command dump

[Marco Gerards]

Bean <[EMAIL PROTECTED]> writes:

Hi,

> I have written a new command dump, it can hex dump content of file or memory.

Great!

> usage:
>
> dump [-s skip] [-n length] { FILE | (mem) }
>
> The output look just like command hexdump.
>
> If you use (mem) as filename, it will dump physical memory instead of
> file. when dumping memory, the default length is 256, but when dumping
> normal file, the default length is the whole file.
>
> Inside grub2, modules can call function dump directly:
>
> void dump(unsigned long base,char* buf, int len);
>
> -- 
> Bean
>
> 2007-08-03  Bean  <[EMAIL PROTECTED]>
>
>   * conf/common.rmk (pkgdata_MODULES): Add dump.mod.
>   (dump_mod_SOURCES): New variable.
>   (dump_mod_CFLAGS): Likewise.
>   (dump_mod_LDFLAGS): Likewise.
>
>   * include/grub/dump.h: New file.
>
>   * commands/dump.c: New file.

I think we should commit this soon!  Can you please add it to grub-emu
as well?

> Index: conf/common.rmk
> ===
> RCS file: /sources/grub/grub2/conf/common.rmk,v
> retrieving revision 1.16
> diff -u -p -r1.16 common.rmk
> --- conf/common.rmk   2 Aug 2007 19:12:52 -   1.16
> +++ conf/common.rmk   3 Aug 2007 14:59:55 -
> @@ -199,7 +199,7 @@ lvm_mod_LDFLAGS = $(COMMON_LDFLAGS)
>  pkgdata_MODULES += hello.mod boot.mod terminal.mod ls.mod\
>   cmp.mod cat.mod help.mod font.mod search.mod\
>   loopback.mod configfile.mod \
> - terminfo.mod test.mod blocklist.mod
> + terminfo.mod test.mod blocklist.mod dump.mod
>
>  # For hello.mod.
>  hello_mod_SOURCES = hello/hello.c
> @@ -276,6 +276,11 @@ blocklist_mod_SOURCES = commands/blockli
>  blocklist_mod_CFLAGS = $(COMMON_CFLAGS)
>  blocklist_mod_LDFLAGS = $(COMMON_LDFLAGS)
>
> +# For blocklist.mod.
> +dump_mod_SOURCES = commands/dump.c
> +dump_mod_CFLAGS = $(COMMON_CFLAGS)
> +dump_mod_LDFLAGS = $(COMMON_LDFLAGS)
> +
>  # Misc.
>  pkgdata_MODULES += gzio.mod elf.mod
>
> Index: include/grub/dump.h
> ===
> RCS file: /sources/grub/grub2/include/grub/dump.h,v
> diff -Nu include/grub/dump.h
> --- /dev/null 2007-08-03 21:55:33.6 +0800
> +++ include/grub/dump.h   2007-08-03 18:27:57.0 +0800
> @@ -0,0 +1,25 @@
> +/* dump.h - prototypes for dump */
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2007  Free Software Foundation, Inc.
> + *
> + *  GRUB is free software: you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation, either version 3 of the License, or
> + *  (at your option) any later version.
> + *
> + *  GRUB is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with GRUB.  If not, see .
> + */
> +
> +#ifndef GRUB_DUMP_H
> +#define GRUB_DUMP_H  1
> +
> +void dump(unsigned long bse,char* buf,int len);
> +
> +#endif /* ! GRUB_DUMP_H */
> Index: commands/dump.c
> ===
> RCS file: /sources/grub/grub2/commands/dump.c,v
> diff -Nu commands/dump.c
> --- /dev/null 2007-08-03 21:55:33.6 +0800
> +++ commands/dump.c   2007-08-03 23:00:48.0 +0800
> @@ -0,0 +1,144 @@
> +/* dump.c - command to dump the contents of a file or memory */
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2007  Free Software Foundation, Inc.
> + *
> + *  GRUB is free software: you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation, either version 3 of the License, or
> + *  (at your option) any later version.
> + *
> + *  GRUB is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with GRUB.  If not, see .
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +static const struct grub_arg_option options[] = {
> +  {"skip", 's', 0, "skip offset bytes from the beginning of file.", 0,
> +   ARG_TYPE_INT},
> +  {"length", 'n', 0, "read only length bytes", 0, ARG_TYPE_INT},
> +  {0, 0, 0, 0, 0, 0}
> +};
> +
> +void
> +dump (unsigned long bse, char *buf, int len)

Can you please use another name, this is a not static.  Not being
static is not a problem to me, having access f

Re: NTFS file system driver (update 4)

[Marco Gerards]

Bean <[EMAIL PROTECTED]> writes:

> On 8/3/07, Marco Gerards <[EMAIL PROTECTED]> wrote:
>> Bean <[EMAIL PROTECTED]> writes:
>>
>> > On 8/3/07, Marco Gerards <[EMAIL PROTECTED]> wrote:
>> >> Bean <[EMAIL PROTECTED]> writes:
>> >>
>> >> >> If you make these two changes and send in the patch, I'll commit the
>> >> >> patch.
>> >> >>
>> >> >
>> >> > Ok, this is the new patch.
>> >>
>> >> Committed.
>> >>
>> >
>> > Thanks, but i don't see fs/ntfs.c in the cvs, do you forget to add it ?
>>
>> Yes, I forgot it, but now it is really committed.
>>
>> > As ntfs driver is done, i start to work on the scripting support. Does
>> > the parser in the previous test program ok ?
>>
>> To be honest, I just didn't find the time yet :(.
>>
>> Actually, I hoped you could first update the other patch regarding
>> scripting.  There was one part of the patch I didn't like.  If you can
>> take that out, at least the other parts can be committed without much
>> of a problem.
>
> Which part ? Do you mean the new lexer code in grub_script_yylex2 ?

Yes.

>> About that other parser file?  Personally I don't like it if you would
>> start from working from scratch on the parser.  The way ASTs and stuff
>> are build now is just fine, although not perfect.  However, if you
>> improve the current parsing rules that would be great.  I do not have
>> much time today, so if I will not have the time today, I'll try to
>> look after this weekend.
>>
>
> Yes, we can just use the parsing rules, AST and related stuff will
> remain the same.
>
> But I think the lexer code need to be changed, the current lexer will
> break the following token:
>
> "aa$BB"
>
> into aa and $BB, but in fact, it should act as one.

I will look into this problem soon.  At the time I just ignored it to
speed up the implementation, sorry for that.

Do you object if the other part is applied?  I would appreciate it if
you had the time to make a new patch...

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Transparent decompression with file system filter

[Marco Gerards]

Bean <[EMAIL PROTECTED]> writes:

> Currently, grub2 support gziped file with the gzio module. To open a
> gziped file,
> you have to use a special function grub_gzfile_open, I think it could
> be better if
> grub_file_open could handle compressed file transparently.

Are you still interested in working on this?

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] bug fix for the ntfs driver

[Marco Gerards]

Bean <[EMAIL PROTECTED]> writes:

Hi,

> The problem is caused by the structure of compressed files. In ntfs,
> 16 clusters is the basic compressed unit. The original code assume
> that the file will break in 16 cluster long blocks, but in fact, the
> blocks will be merged if they are stored next to to each other on
> disk. The new code fix this problem.
>
>* fs/ntfs.c (read_block): Fix a bug caused by adjacent blocks.

Sorry for forgetting about this patch.  It should be committed.

Robert, if you have time, can you commit this?  I will mark this mail
as important... If you don't have the time, I will surely commit it
ASAP :-)

--
Marco





___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] generic ELF version of grub-mkimage

[Marco Gerards]

Robert Millan <[EMAIL PROTECTED]> writes:

> On Fri, Oct 12, 2007 at 05:55:03PM +0200, Robert Millan wrote:
>> 
>> Ok.  But no, I didn't really notice.  I'll fix this entry and keep trying to
>> get it right next time.
>
> Does this look good?

This was committed, right?  Otherwise poke me and I will look :-)

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: error: "no free pages available"?

[Marco Gerards]

Charles Turner <[EMAIL PROTECTED]> writes:

> Wow, tough crowd on this list...

It seemed we all picked the same time to be very busy :-/

> Pursuant to my previous email, I placed grub_printf() statements at 
> approximately line #351 and #606 in the file loader/i386/efi/linux.c. 
> That would be where grub_rescue_cmd_linux() and 
> grub_rescue_cmd_initrd() get their ideas about what initrd_addr_max 
> should be.
>
> In both cases, 0x1fff is returned, which is a value carved into the 
> kernel header of my vmlinuz file. So Grub2 is doing its job, and it 
> seems there's some conflict between where initrd memory is on my Core 2 
> Duo MacMini (@ around 1gb), and where the Debian AMD64 
> vmlinuz-2.6.21-2-686 kernel is saying it can't be found above.
>
> I don't have any experience compiling the kernel as I've not so far had 
> a reason to. :-)
>
> Is initrd_addr_max a configurable piece of the kernel? Again, not 
> really sure right now where to take this info...

Recently, some patches for EFI were sent in.  Did these fix your
problem?  If not, please keep poking us... We might wake up ;)

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Strong Crypto Support for GRUB2

[Marco Gerards]

Simon Peter <[EMAIL PROTECTED]> writes:

Hi,

> is my patch for strong crypto support going to make it into GRUB2
> mainline?

Sorry for keeping you waiting.  I hope you are still interested in
this; I am.

Can you please send in your work as a patch+changelog to current CVS?
Please make clear what you wrote and where other code comes from.  We
usually deal with copyright issues very carefully.

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: pxebooting with GRUB 2

[Marco Gerards]

Daniel Kahn Gillmor <[EMAIL PROTECTED]> writes:

> On Sun 2007-09-30 12:07:38 -0400, Robert Millan wrote:
>
>> WIP.  Check latest mails from Marco on the subject.  He sent patches
>> implementing ethernet infrastructure and requested feedback.
>
> Ah, thanks.  Are you referring to this, from two months ago?
>
>  http://lists.gnu.org/archive/html/grub-devel/2007-08/msg00011.html
>
> I couldn't find a more recent e-mail.  And though the message suggests
> that the patchset will be in CVS within a week (i.e. mid-August), it
> doesn't look merged to me.  i'm looking at CVS HEAD, though -- is
> there another branch i should be trying?

It will be committed soon, now for real ;-)

> I'm not sure what feedback i can give as i'm not a grub developer, and
> there doesn't appear to be enough in the patch (e.g. no IPv4 support)
> to make it something i can test as an end user.

No, you are right.  In the current state it will not be very useful.
I will work on this after dealing with patches that were sent in,
LinuxBIOS related work and ATA support.

> Marco, can you give another status report on netbooting?

This patch that will be committed soon will be the foundation of
networking support.  After that I will add IPv4, UDP, TFTP, etc.

--
Marco




___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Grub2 on a Macbook.

[Marco Gerards]

Isaac Dupree <[EMAIL PROTECTED]> writes:

> Eoin Hennessy wrote:
>> Hello all,
>>
>> I've been taking a look at grub2 on an EFI Apple Macbook. I've
>> installed a build from CVS HEAD and I can chainload grub.efi via the
>> rEFIt loader. When grub loads I'm seeing the 'grub rescue>' command
>> prompt. As a test, I'd like to chainload OSX's boot.efi using the
>> 'chainloader' command listed at [0] but the command is reported as
>> 'unknown'. I've also tried the instructions at [1] but it allooks like
>> grub.cfg is being ignored. Perhaps someone could point me in the right
>> direction.
>
> In May/June 2007 I failed at using Grub2 cvs - efi on my new MacBook
> too, with the same issue.  But I'm using Grub2 with legacy boot
> sequence to boot... because it understands GPT :)  and that works
> except the keyboard usually doesn't work during boot because of
> Apple's buggy bios.

Are you aware of other bootloaders that do *not* have this problem?  I
experienced this with GRUB Legacy.  The workaround was not pretty...

> I'm not sure if chainloading would work due to grub2 not understanding
> HFS+ ?  I tried putting grub2-efi and a linux kernel image on a
> FAT16/vfat partition.

It *does* understand HFS+, if you load the hfsplus.mod module :-)

> Anyway I can probably help test anything if needed.

Can you please send in bug reports whenever something in GRUB 2
doesn't work?  Or better: document it on the wiki so we will not
forget about this :-)

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Fixed ieee1275 console

[Marco Gerards]

"Marcin Kurek" <[EMAIL PROTECTED]> writes:

> Hell[o]
>
>> Right, this is because it seems that the MSB is lost when writing a
>> character to the screen.  For the borders specific characters are used
>> above 127 (non-ASCII).  I wonder if the Pegasos console supports this
>> ASCII extension if you change some settings or so...
>
> The problem is in framebuffer mode OF loads glyphs starting from 32 to
> 127 then any characters abowe 127 are displayed as white '?'. The
> solution would be to use "set-font" to load a proper font to use with
> grub. In non framebuffer mode cp437 is used by console then we can use
> it to draw borders same as for PC.

Can you provide something that makes use of that?

>> I prefer if it can be detected if this is Efika or for example an
>> apple implementation of OF and handle these characters depending on
>> that.  IIRC this code does work on the apple, but unfortunately I
>> cannot check this anymore.
>
> Ahhh, I see you looked at first version of patch. Please take a look
> at recent version of my patches.

Will do :-)

>> > Handle 127 keycode as backspace key in grub_ofconsole_readkey() which fix
>> [1]
>>
>> Can you explain this?
>
> For some reasons my OF sends \b && 127 (Del) sequence for backspace
> key. It seems to be same sa ncurses console.

Ok.

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Use getopt_long() instead of argp_parse() in grub-emu

[Marco Gerards]

Christian Franke <[EMAIL PROTECTED]> writes:

> Unlike the other GRUB2 utils, grub-emu uses the glibc extension
> argp_parse(). It is unavailable on Cygwin, which might also be the
> case for other platforms where glibc is not the native runtime.

This has been brought up before, BSD has the same problem.  The
outcome of the discussion was (IIRC) that we will use argp.  When argp
is not available we can use gnulib or a standalone argp parser
("argp-standalone") to support this.  In that case it will mean
changing configure.ac.

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: grub-setup: error: Non-sector-aligned data is found in the core file

[Marco Gerards]

Robert Millan <[EMAIL PROTECTED]> writes:

> On Thu, Nov 08, 2007 at 02:36:28PM +0200, UrJiZ wrote:
>> On 11/7/07, Robert Millan <[EMAIL PROTECTED]> wrote:
>> 
>> > For building a floppy use grub-mkrescue instead.
>> >
>> 
>> Used ./grub-mkrescue, but it:
>> a) seems not to create a filesystem on the disk image (how can i put
>> grub.cfg on the disk?)
>
> I was thinking we could have some way to embed filesystems in a GRUB image
> (we might need this for LinuxBIOS port).

We could just add files using grub-mkimage.  If a file isn't a module,
it won't be loaded but accessible.  We can use (host) or so for
this...

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Outline menu

[Marco Gerards]

Markus Elfring <[EMAIL PROTECTED]> writes:

>> Menu commands can have children already so you would get hierarchy
>> information from there. Shortcut key you could just add as extra
>> argument for menu, like --hotkey="1"
>
> Will an explanation of the data format be added to the wiki?
>
>
>> In example:
>> menu "foo" {
>>   menu "bar" {
>> menu "zot" {
>> }
>>   }
>> }
>
> Can an item be reused at different levels in the hierarchy without repetition 
> of
> its definition (attribute set)?

I want to make something to generate menu entries from a script.  Is
that what you mean?

>> Though I am not sure how well this outline view is supported by the
>> current parser. Marco, perhaps you could share some insights?
>
> How is the state of the GRUB 2 menu browser at the moment?

It's not stable yet.  Especially the scripting needs to be worked on.

>> I had a look at example mp3 playlist in opml format and it seems to be
>> data description language (like what xml is). What we have in grub
>> script is a scripting language that has commands to construct data (menu).
>
> Is a manual available already?

No, do you want to work on this?

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Outline menu

[Marco Gerards]

Vesa Jääskeläinen <[EMAIL PROTECTED]> writes:

> Markus Elfring wrote:
>>> And what is wrong with current config file?
>>> http://grub.enbug.org/grub.cfg
>> 
>> How much documentation is available for this domain-specific language?
>> Does a grammar exist for the data format?
>
> Marco Gerards has been working on the grub scripting language. So he
> might be able to give some insights in case Wiki does not feature enough
> information.

It's supposed to be bash like :)

>> http://grub.enbug.org/Subprojects/GraphicalMenu
>> 
>> I imagine that additional attributes will be needed.
>> - Identifier: optional alphanumeric key
>> - Parent: index number or menu identifier
>
> Menu commands can have children already so you would get hierarchy
> information from there. Shortcut key you could just add as extra
> argument for menu, like --hotkey="1"
>
> In example:
>
> menu "foo" {
>   menu "bar" {
> menu "zot" {
> }
>   }
> }
>
> Though I am not sure how well this outline view is supported by the
> current parser. Marco, perhaps you could share some insights?

Personally I first want to focus on getting what we have to work,
before even thinking about anything else.  Ideas are good, but having
*something* to use is better :-)

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: How do I boot a standalone program in grub (a password checker via serial input) before allowing grub to continue booting?

[Marco Gerards]

CurlyCoconutTree <[EMAIL PROTECTED]> writes:

> How do I boot a standalone program in grub (1.95) (a password checker via
> serial input) before allowing grub to continue booting?  I'm trying to
> develop a program that will run, stand alone or with the libraries Grub
> gives me access to, parse serial data for a password, store the password to
> the MBR, then if valid return to Grub to boot windows... I'm not looking for
> a solution, I'm just looking for a place to start!  Grub, is pretty poorly
> documented and I wouldn't be posting this is there was more (or I could find
> more) info on how to develop for GRUB.  I guess I'd be a 'noob'... I hate
> using that term... we all have to start from some where.

Noone is using that term.  We also value good documentation and will
be very happy if you could work on that.

One way of doing this might be writing a new command.  See
hello/hello.c as an example.

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Grub 2 functionality

[Marco Gerards]

Scott A Tovey <[EMAIL PROTECTED]> writes:

Hi,

> I have two questions.
>
> 1. Will grub2 be taking over the hard drive
> recognition from the BIOS?

It can be done, but it is not preferred.  We are working on this.

> I believe that the Max OS X boots without using
> the BIOS hard drive IO. This allows the OS to
> boot on any hardware regardless of whether the
> BIOS can recognize the entire drive or not.

It uses EFI.  It's still using firmware, just a different kind of
firmware.  GRUB 2 is capable of running on EFI.

> If GRUB had this functionality and provided the
> hard drive parameters to the OS instead of the
> BIOS, it would enable many older systems that do
> not have a flashable BIOS to be upgraded with
> newer hard drives and utilized as low grade
> servers or backup storage devices.

We should all switch to LinuxBIOS someday. ;-)

If you are using GNU/Linux, GRUB2+ATA can be used on a PC BIOS.

> Besides the benefit to older hardware, it would
> be good for Linux and other OS' that chose to use
> GRUB as the boot loader.

Not if that older OS uses the BIOS.  Because GRUB can access the
harddisk and load the OS, but if the OS can't access the hardware it
will not work.

> 2. Is there an installer that would allow me to
> install GRUB on my system without having to
> install Linux?

You could use a floppy/CDROM.  From which OS do you want to install?

--
Marco




___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Add host open devicename check

[Marco Gerards]

Robert Millan <[EMAIL PROTECTED]> writes:

> On Fri, Nov 09, 2007 at 10:17:19PM +0100, Marco Gerards wrote:
>> Robert Millan <[EMAIL PROTECTED]> writes:
>> 
>> > On Thu, Oct 25, 2007 at 09:51:38PM +0200, Christian Franke wrote:
>> >>  static grub_err_t
>> >> -grub_host_open (const char *name __attribute((unused)), grub_disk_t disk)
>> >> +grub_host_open (const char *name, grub_disk_t disk)
>> >>  {
>> >> +  if (grub_strcmp(name, "host"))
>> >> +  return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "not a host disk");
>> >> +
>> >
>> > I would test for (disk->id == GRUB_DISK_DEVICE_HOST_ID) instead.  Faster,
>> > and also cleaner/simpler IMHO.
>> 
>> It's not possible unfortunately :-(.  This information is about to be
>> filled in in this same function.
>
> Still seems like an ugly hack to me.  Oh well :-/

It isn't.  The driver gets a string that it can use to determine if it
has control over this disk or not.  `grub_disk_t disk' is there to be
filled in if it has, it isn't initialised yet.

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Add host open devicename check

[Marco Gerards]

Christian Franke <[EMAIL PROTECTED]> writes:

> Marco Gerards wrote:
>>> ..
>>>
>>> 2007-10-25  Christian Franke  <[EMAIL PROTECTED]>
>>>
>>> * disk/host.c (grub_host_open): Add check for "host". This fixes
>>> the problem that grub-emu does not find partitions.
>>>
>>
>> Please mention the attribute change.
>>
>>
>>> ...
>>>  static grub_err_t
>>> -grub_host_open (const char *name __attribute((unused)), grub_disk_t disk)
>>> +grub_host_open (const char *name, grub_disk_t disk)
>>>  {
>>> +  if (grub_strcmp(name, "host"))
>>> +  return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "not a host disk");
>>>
>>
>> Please add a space after the function name and before the "(".
>>
>>
>
> Done
>
> Christian
>
> 2007-11-10  Christian Franke  <[EMAIL PROTECTED]>
>
>   * disk/host.c (grub_host_open): Remove attribute unused from
>   name parameter. Add check for "host". This fixes the problem
>   that grub-emu does not find partitions.

Looks fine to me.

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Allow build without dirent.d_type, fix "ls (host)" crash

[Marco Gerards]

Christian Franke <[EMAIL PROTECTED]> writes:

> Marco Gerards wrote:
>
>>> ...
>>> +static int
>>> +is_dir(const char *path, const char *name)
>>>
>>
>> is_dir (
>>
>>
>
> OK. Too many projects, too many policies, sorry :-)

np, don't worry :-)

Hopefully you are not annoyed by my comments.  A consistent style
throughout GRUB is important to me.

>>> +{
>>> +  int len1 = strlen(path), len2 = strlen(name);
>>>
>>
>> Please split this up.  Or even better use separate declarations and
>> code.
>>
>>
>
> Yes. No. See comment below.
>
>>> +  char pathname[len1+1+len2+1+13];
>>>
>>
>> Please add spaces around binary operators.
>>
>>
>>> +  struct stat st;
>>> +  strcpy (pathname, path);
>>>
>>
>> Please add more blank lines between your code to make it easier to
>> read. :-)
>>
>>
>
> OK.
>
>
>>> ...
>>> +strcat (pathname, "/");
>>> +  strcat (pathname, name);
>>> +  if (stat (pathname, &st))
>>> +return 0;
>>> +  return S_ISDIR(st.st_mode);
>>> +}
>>> +#endif
>>>
>>
>> Why can't you just use S_ISDIR?
>>
>>
>
> ??

Nevermind, I wasn't really awake ;)

>>> ...
>>> @@ -81,6 +108,7 @@ grub_hostfs_read (grub_file_t file, char
>>>FILE *f;
>>> f = (FILE *) file->data;
>>> +  fseek (f, file->offset, SEEK_SET);
>>>int s= fread (buf, 1, len, f);
>>>
>>
>> "int s = "
>>
>>
>
> Code janitor work outside the scope of this patch ... done ;-)

LOL!  Sorry, I was a bit enthousiastic ;-)

Thanks! :)

> BTW: This line uses "declaration statement" syntax, therefore this is
> apparently accepted in GRUB2 codebase :-)
> I definitely prefer this (first use = declaration = initialization)
> style, which is state of the art in most (all?) modern languages.
> This C++ (1986) feature and early GCC (1.*) extension was finally
> included into C99, so it is portable now.
> is_dir() rewritten accordingly.

It is.  The problem is that you had two function calls on one line.  I
want to avoid that.

> Christian
>
> 2007-11-09  Christian Franke  <[EMAIL PROTECTED]>
>
>   * util/hostfs.c (is_dir): New function.
>   (grub_hostfs_dir):  Handle missing dirent.d_type case.
>   (grub_hostfs_read): Add missing fseek().
>   (grub_hostfs_label): Clear label pointer.  This fixes a crash
>   of grub-emu on "ls (host)".

Looks fine to me.

btw, when do you use (host)?  It's intended as a debugging tool.

--
Marco




___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Fix eisa_mmap evaluation, add memory existence check

[Marco Gerards]

Christian Franke <[EMAIL PROTECTED]> writes:

> Marco Gerards wrote:
>> ...
>>> +static int
>>> +addr_is_valid (grub_addr_t addr)
>>> +{
>>> +  volatile unsigned char * p = (volatile unsigned char *)addr;
>>>
>>
>> Why volatile?  I have the feeling it is not needed.
>>
>>
>>> +  unsigned char x, y;
>>> +  x = *p;
>>> +  *p = x ^ 0xcf;
>>> +  y = *p;
>>> +  *p = x;
>>> +  return y == (x ^ 0xcf);
>>> +}
>>>
>>
>>
>
> volatile is necessary here to tell the complier that the memory
> address might not behave like regular memory. Otherwise, the optimizer
> might legitimately remove memory accesses and then constant
> propagation detects an unchanged value.
>
> gcc actually does a very good job here. Result with volatile removed:

I think this is just normal memory, not memory mapped hardware.  Or
perhaps I am misunderstanding something here.

>
> $ gcc -S -O -fomit-frame-pointer init.c && cat init.s
> ...
> addr_is_valid:
>movl$1, %eax
>ret
> ...
>
>
> aka:
>
> static int
> addr_is_valid (grub_addr_t addr)
> {
>  return 1;
> }
>
>
> This is at least a proof that the original function returns the
> correct result when real memory is present :-)

:-)

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Fix packing issue of machine_mmap_entry

[Marco Gerards]

Christian Franke <[EMAIL PROTECTED]> writes:

> Marco Gerards wrote:
>
>>> ...
>>> Add compile time assert to check packing.
>>>
>>
>> Can you remove the compile time assert?
>
> Done.
>
>> We usually check stuff like
>> this using configure.  If you can send in a patch for configure.ac,
>> that would be appreciated.
>>
>>
> Yes, but be patient.
>
> The configure approach is quite different: In configure, you can check
> whether compiler supports attr packed and whether it works as
> expected. With the good old typedef compile time assert, you can check
> whether this *specific* structure is properly declared and packed.

Right, but if you add a configure.ac option you can see if it works
globally.  If it doesn't, we have a problem anyways.  We shouldn't
rely on how it is packed.  If the packed attribute doesn't work, GRUB
can't be compiled.

>>> --- grub2.orig/include/grub/i386/pc/init.h  2007-07-22 01:32:23.0 
>>> +0200
>>> +++ grub2/include/grub/i386/pc/init.h   2007-10-13 21:25:24.0 
>>> +0200
>>> @@ -40,10 +40,14 @@ grub_uint32_t grub_get_eisa_mmap (void);
>>>  struct grub_machine_mmap_entry
>>>  {
>>>grub_uint32_t size;
>>> -  grub_uint64_t addr;
>>> +  grub_uint64_t addr; /* must be at offset 4, see startup.S */
>>
>> I do not think this comment is required.  It's fixed now :-)
>>
>>
>
> Hmm...OK removed. Now there is no clue why this struct has packing
> requirements. And this also is no longer checked.

All structs that rely on gcc not adding packing for alignment, should
have the packed attribute.

> Christian
>
>
> 2007-11-09  Christian Franke  <[EMAIL PROTECTED]>
>
>   * include/grub/i386/pc/init.h (struct grub_machine_mmap_entry):
>   Add attribute packed, gcc 3.4.4 on Cygwin aligns this
>   to 64 bit boundary by default.

This looks good :-)

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: grub2 efi patches

[Marco Gerards]

Alexandre Boeglin <[EMAIL PROTECTED]> writes:

Hi,

> Here are a few patches for grub2, against cvs head:

Great!  Thanks for working on this!  Can you please send in ChangeLog
entries for these patches?

> grub2_arg_doubledash.patch fixes a bug in the handling of the '--'
> argument,
>
> grub2_efi_chainloader_options.patch adds support for efi chainload options,
> it is not really beautiful (for instance, the ascii to utf16 conversion),
> but it works for the MacOSX loader,
>
> grub2_efi_grub_prefix.patch uses the grub_prefix variable to set the prefix
> environment variable, instead of the directory in which grub.efi is. This
> allows to have grub.efi and the modules in different folders, and the old
> behaviour should be preserved, if the grub_prefix is an empty string,

How about passing it as an argument to GRUB 2?  I assume EFI can do
this?  This is what we do on open firmware, IIRC.

> grub2_efi_halt_reboot.patch adds support for the reboot and halt commands,
> provided by efi runtime services.
>
> Regards,
> Alex
>
> P.S.: I sent the patches as attachments, I don't know if they will be
> discarded by the list robot or not ...

No, this is even preferred.  Could you use "diff -up" for future
patches?

> Alexandre Boeglin
> email: alex (at) boeglin (dot) org
> jabber: alex (at) im (dot) apinc (dot) org
> website: http://boeglin.org/
>
> Index: normal/arg.c
> ===
> RCS file: /sources/grub/grub2/normal/arg.c,v
> retrieving revision 1.7
> diff -u -r1.7 arg.c
> --- normal/arg.c  21 Jul 2007 23:32:28 -  1.7
> +++ normal/arg.c  3 Nov 2007 22:44:41 -
> @@ -313,7 +313,7 @@
> if (grub_strlen (arg) == 2)
>   {
> for (curarg++; curarg < argc; curarg++)
> - if (add_arg (arg) != 0)
> + if (add_arg (argv[curarg]) != 0)
> goto fail;
> break;
>   }

Looks fine to me.

> Index: include/grub/efi/chainloader.h
> ===
> RCS file: /sources/grub/grub2/include/grub/efi/chainloader.h,v
> retrieving revision 1.2
> diff -u -r1.2 chainloader.h
> --- include/grub/efi/chainloader.h21 Jul 2007 23:32:22 -  1.2
> +++ include/grub/efi/chainloader.h3 Nov 2007 22:42:01 -
> @@ -19,6 +19,6 @@
>  #ifndef GRUB_EFI_CHAINLOADER_HEADER
>  #define GRUB_EFI_CHAINLOADER_HEADER  1
>  
> -void grub_chainloader_cmd (const char *filename);
> +void grub_chainloader_cmd (int argc, char *argv[]);
>  
>  #endif /* ! GRUB_EFI_CHAINLOADER_HEADER */
> Index: loader/efi/chainloader.c
> ===
> RCS file: /sources/grub/grub2/loader/efi/chainloader.c,v
> retrieving revision 1.2
> diff -u -r1.2 chainloader.c
> --- loader/efi/chainloader.c  21 Jul 2007 23:32:28 -  1.2
> +++ loader/efi/chainloader.c  3 Nov 2007 22:42:47 -
> @@ -17,8 +17,6 @@
>   *  along with GRUB.  If not, see .
>   */
>  
> -/* TODO: support load options.  */
> -
>  #include 
>  #include 
>  #include 
> @@ -175,7 +173,7 @@
>  }
>  
>  void
> -grub_chainloader_cmd (const char *filename)
> +grub_chainloader_cmd (int argc, char *argv[])
>  {
>grub_file_t file = 0;
>grub_ssize_t size;
> @@ -185,6 +183,11 @@
>grub_device_t dev = 0;
>grub_efi_device_path_t *dp = 0;
>grub_efi_loaded_image_t *loaded_image;
> +  char *filename = argv[0];
> +  grub_efi_char16_t **options = NULL, *p;
> +  int i, j, options_len = 0;
> +
> +  b = grub_efi_system_table->boot_services;
>
>grub_dl_ref (my_mod);
>  
> @@ -192,6 +195,30 @@
>address = 0;
>image_handle = 0;
>file_path = 0;
> +
> +  if (argc == 0)
> +  {
> +grub_error (GRUB_ERR_BAD_ARGUMENT, "no file specified");
> +goto fail;
> +  }

This indentation doesn't look right.  Same for the other "if"s.

> +  /* copy options */
> +  if (argc > 1)
> +  {
> +for (i = 1; i < argc; i++)
> +  options_len += (grub_strlen (argv[i]) + 1) * sizeof (**options);
> +
> +status = b->allocate_pool (GRUB_EFI_LOADER_DATA, options_len + sizeof 
> (**options), options);

Do you deallocate when an error occurs?

> +if (status != GRUB_EFI_SUCCESS)
> +  goto fail;
> +p = *options;
> +
> +for (i = 1; i < argc; i++){
> +  *p++ = ' ';
> +  for (j = 0; j < grub_strlen (argv[i]) + 1; j++)
> +p[j] = argv[i][j];
> +}

This indentation is not right.

> +  }
>
>b = grub_efi_system_table->boot_services;
>  
> @@ -267,6 +294,10 @@
>goto fail;
>  }
>loaded_image->device_handle = dev_handle;
> +
> +  if (*options)
> +loaded_image->load_options = *options;
> +loaded_image->load_options_size = options_len + 1;

This indentation is weird.  Did you forget { and }?  That's what this
indentation suggests.

>grub_file_close (file);
>grub_loader_set (grub_chainloader_boot, grub_chainloader_unload, 0);
> @@ -292,10 +323,7 

Re: grub2 efi patches

[Marco Gerards]

Alexandre Boeglin <[EMAIL PROTECTED]> writes:

> On Sun, 4 Nov 2007 01:21:24 +0100, Alexandre Boeglin <[EMAIL PROTECTED]>
> wrote:
>> Le Sun, Nov 04, 2007 at 01:04:24AM +0100, Alexandre Boeglin a écrit :
>>> Sorry, there was a mistake in the previous one ...
>> Oops, and line 91 of this one should be
>
> Hi, here is a "more correct" version of this patch: it also frees the
> memory allocated to the options string when appropriate.

I should've started with this patch ;-)

I can better do a full review again for clarity :-)

> Index: include/grub/efi/chainloader.h
> ===
> RCS file: /sources/grub/grub2/include/grub/efi/chainloader.h,v
> retrieving revision 1.2
> diff -u -r1.2 chainloader.h
> --- include/grub/efi/chainloader.h21 Jul 2007 23:32:22 -  1.2
> +++ include/grub/efi/chainloader.h5 Nov 2007 22:58:51 -
> @@ -19,6 +19,6 @@
>  #ifndef GRUB_EFI_CHAINLOADER_HEADER
>  #define GRUB_EFI_CHAINLOADER_HEADER  1
>  
> -void grub_chainloader_cmd (const char *filename);
> +void grub_chainloader_cmd (int argc, char *argv[]);
>  
>  #endif /* ! GRUB_EFI_CHAINLOADER_HEADER */
> Index: loader/efi/chainloader.c
> ===
> RCS file: /sources/grub/grub2/loader/efi/chainloader.c,v
> retrieving revision 1.2
> diff -u -r1.2 chainloader.c
> --- loader/efi/chainloader.c  21 Jul 2007 23:32:28 -  1.2
> +++ loader/efi/chainloader.c  5 Nov 2007 22:58:58 -
> @@ -17,8 +17,6 @@
>   *  along with GRUB.  If not, see .
>   */
>  
> -/* TODO: support load options.  */
> -
>  #include 
>  #include 
>  #include 
> @@ -38,6 +36,8 @@
>  
>  static grub_efi_physical_address_t address;
>  static grub_efi_uintn_t pages;
> +static grub_efi_char16_t *options;
> +static grub_efi_uintn_t options_len;
>  static grub_efi_device_path_t *file_path;
>  static grub_efi_handle_t image_handle;
>  
> @@ -49,6 +49,8 @@
>b = grub_efi_system_table->boot_services;
>b->unload_image (image_handle);
>b->free_pages (address, pages);
> +  if (options)
> +b->free_pool (options);
>grub_free (file_path);
>
>grub_dl_unref (my_mod);
> @@ -175,7 +177,7 @@
>  }
>  
>  void
> -grub_chainloader_cmd (const char *filename)
> +grub_chainloader_cmd (int argc, char *argv[])
>  {
>grub_file_t file = 0;
>grub_ssize_t size;
> @@ -185,16 +187,47 @@
>grub_device_t dev = 0;
>grub_efi_device_path_t *dp = 0;
>grub_efi_loaded_image_t *loaded_image;
> +  char *filename = argv[0];
>
>grub_dl_ref (my_mod);
>  
>/* Initialize some global variables.  */
>address = 0;
> +  options = 0;
> +  options_len = 0;
>image_handle = 0;
>file_path = 0;
>
>b = grub_efi_system_table->boot_services;
>  
> +  if (argc == 0)
> +  {
> +grub_error (GRUB_ERR_BAD_ARGUMENT, "no file specified");
> +goto fail;
> +  }

The indentation is not right.

> +  /* copy options */

/* Copy options.  */

> +  if (argc > 1)
> +  {

The indentation is not right.

> +grub_efi_char16_t *p;
> +grub_efi_int16_t i;
> +grub_efi_uint16_t j;
> +for (i = 1; i < argc; i++)
> +  options_len += (grub_strlen (argv[i]) + 1) * sizeof (*options);
> +options_len += sizeof (*options); /* \0 */
> +
> +status = b->allocate_pool (GRUB_EFI_LOADER_DATA, options_len, (void *) 
> &options);
> +if (status != GRUB_EFI_SUCCESS)
> +  goto fail;
> +p = options;

Please use grub_malloc here, I think it should work for what you are
doing, right?

> +for (i = 1; i < argc; i++){
> +  *p++ = ' ';
> +  for (j = 0; j < grub_strlen (argv[i]) + 1; j++)
> +p[j] = (grub_efi_char16_t) argv[i][j];
> +}
> +  }
> +
>file = grub_file_open (filename);
>if (! file)
>  goto fail;
> @@ -268,6 +301,10 @@
>  }
>loaded_image->device_handle = dev_handle;
>
> +  if (options_len)
> +loaded_image->load_options = options;
> +loaded_image->load_options_size = options_len;

The second loaded_image... line seems to be incorrectly indented.

>grub_file_close (file);
>grub_loader_set (grub_chainloader_boot, grub_chainloader_unload, 0);
>return;
> @@ -286,16 +323,16 @@
>if (address)
>  b->free_pages (address, pages);
>
> +  if (options)
> +b->free_pool (options);
> +
>grub_dl_unref (my_mod);
>  }
>  
>  static void
>  grub_rescue_cmd_chainloader (int argc, char *argv[])
>  {
> -  if (argc == 0)
> -grub_error (GRUB_ERR_BAD_ARGUMENT, "no file specified");
> -  else
> -grub_chainloader_cmd (argv[0]);
> +  grub_chainloader_cmd (argc, argv);
>  }
>  
>  static const char loader_name[] = "chainloader";
> Index: loader/efi/chainloader_normal.c
> ===
> RCS file: /sources/grub/grub2/loader/efi/chainloader_normal.c,v
> retrieving revision 1.2
> diff -u -r1.2 chainloader_normal.c
> --- loader/efi/chainloader_normal.c   21 Jul 2007 23:

Re: grub2 efi patches

[Marco Gerards]

Alexandre Boeglin <[EMAIL PROTECTED]> writes:

> On Sun, 4 Nov 2007 00:06:31 +0100, Alexandre Boeglin <[EMAIL PROTECTED]>
> wrote:
>> grub2_efi_chainloader_options.patch adds support for efi chainload
>> options,
>> it is not really beautiful (for instance, the ascii to utf16 conversion),
>> but it works for the MacOSX loader,
>
> Sorry, there was a mistake in the previous one ...

Ok :-)

> By the way, it's using boot_services->allocate_pool instead of grub_malloc.
> I don't know if it's okay, but when I tried using grub_malloc, I was
> getting "free magic is broken 0x0" errors.

Please use grub_malloc.  If you get this error, something is most
likely writing to unallocated memory or something like that...

> Index: include/grub/efi/chainloader.h
> ===
> RCS file: /sources/grub/grub2/include/grub/efi/chainloader.h,v
> retrieving revision 1.2
> diff -u -r1.2 chainloader.h
> --- include/grub/efi/chainloader.h21 Jul 2007 23:32:22 -  1.2
> +++ include/grub/efi/chainloader.h4 Nov 2007 00:00:40 -
> @@ -19,6 +19,6 @@
>  #ifndef GRUB_EFI_CHAINLOADER_HEADER
>  #define GRUB_EFI_CHAINLOADER_HEADER  1
>  
> -void grub_chainloader_cmd (const char *filename);
> +void grub_chainloader_cmd (int argc, char *argv[]);
>  
>  #endif /* ! GRUB_EFI_CHAINLOADER_HEADER */
> Index: loader/efi/chainloader.c
> ===
> RCS file: /sources/grub/grub2/loader/efi/chainloader.c,v
> retrieving revision 1.2
> diff -u -r1.2 chainloader.c
> --- loader/efi/chainloader.c  21 Jul 2007 23:32:28 -  1.2
> +++ loader/efi/chainloader.c  4 Nov 2007 00:01:05 -
> @@ -17,8 +17,6 @@
>   *  along with GRUB.  If not, see .
>   */
>  
> -/* TODO: support load options.  */
> -
>  #include 
>  #include 
>  #include 
> @@ -175,7 +173,7 @@
>  }
>  
>  void
> -grub_chainloader_cmd (const char *filename)
> +grub_chainloader_cmd (int argc, char *argv[])
>  {
>grub_file_t file = 0;
>grub_ssize_t size;
> @@ -185,6 +183,11 @@
>grub_device_t dev = 0;
>grub_efi_device_path_t *dp = 0;
>grub_efi_loaded_image_t *loaded_image;
> +  char *filename = argv[0];
> +  grub_efi_char16_t *options = NULL, *p;
> +  int i, j, options_len = 0;
> +
> +  b = grub_efi_system_table->boot_services;
>
>grub_dl_ref (my_mod);
>  
> @@ -192,6 +195,30 @@
>address = 0;
>image_handle = 0;
>file_path = 0;
> +
> +  if (argc == 0)
> +  {
> +grub_error (GRUB_ERR_BAD_ARGUMENT, "no file specified");
> +goto fail;
> +  }
> +
> +  /* copy options */

/* Copy options.  */

> +  if (argc > 1)
> +  {
> +for (i = 1; i < argc; i++)
> +  options_len += (grub_strlen (argv[i]) + 1) * sizeof (*options);
> +
> +status = b->allocate_pool (GRUB_EFI_LOADER_DATA, options_len + sizeof 
> (*options), &options);
> +if (status != GRUB_EFI_SUCCESS)
> +  goto fail;

Please use grub_malloc.

There are several ways to debug mm.c...

> +p = options;
> +
> +for (i = 1; i < argc; i++){
> +  *p++ = ' ';
> +  for (j = 0; j < grub_strlen (argv[i]) + 1; j++)
> +p[j] = argv[i][j];
> +}
> +  }
>
>b = grub_efi_system_table->boot_services;
>  
> @@ -267,6 +294,10 @@
>goto fail;
>  }
>loaded_image->device_handle = dev_handle;
> +
> +  if (options)
> +loaded_image->load_options = options;
> +loaded_image->load_options_size = options_len + 1;
>
>grub_file_close (file);
>grub_loader_set (grub_chainloader_boot, grub_chainloader_unload, 0);
> @@ -292,10 +323,7 @@
>  static void
>  grub_rescue_cmd_chainloader (int argc, char *argv[])
>  {
> -  if (argc == 0)
> -grub_error (GRUB_ERR_BAD_ARGUMENT, "no file specified");
> -  else
> -grub_chainloader_cmd (argv[0]);
> +  grub_chainloader_cmd (argc, argv);
>  }
>  
>  static const char loader_name[] = "chainloader";
> Index: loader/efi/chainloader_normal.c
> ===
> RCS file: /sources/grub/grub2/loader/efi/chainloader_normal.c,v
> retrieving revision 1.2
> diff -u -r1.2 chainloader_normal.c
> --- loader/efi/chainloader_normal.c   21 Jul 2007 23:32:28 -  1.2
> +++ loader/efi/chainloader_normal.c   4 Nov 2007 00:01:05 -
> @@ -26,10 +26,7 @@
>  chainloader_command (struct grub_arg_list *state __attribute__ ((unused)),
>int argc, char **args)
>  {
> -  if (argc == 0)
> -grub_error (GRUB_ERR_BAD_ARGUMENT, "no file specified");
> -  else
> -grub_chainloader_cmd (args[0]);
> +  grub_chainloader_cmd (argc, args);
>return grub_errno;
>  }
>  
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: PC partitions and big disks

[Marco Gerards]

Robert Millan <[EMAIL PROTECTED]> writes:

> On Fri, Nov 09, 2007 at 04:33:47PM +0100, Marco Gerards wrote:
>> Robert Millan <[EMAIL PROTECTED]> writes:
>> 
>> > This should make the error much clearer when partition exceeds 2 TiB on a 
>> > PC
>> > partition map (currently, all user gets is a "out of partition" error which
>> > is hard to figure out if you're unaware of the 2 TiB limit).
>> >
>> > Comments?
>> 
>> See below :-)
>> 
>> > -- 
>> > Robert Millan
>> >
>> >  I know my rights; I want my phone call!
>> >  What use is a phone call, if you are unable to speak?
>> > (as seen on /.)
>> >
>> >
>> >* include/grub/types.h (ULONG_MAX32): New macro.
>> >(ULONG_MAX64): Likewise.
>> >(ULONG_MAX): Use ULONG_MAX32 / ULONG_MAX64 to define it.
>> 
>> Please call use a GRUB_* prefix.
>> 
>> >* util/misc.c (grub_util_warn): New function.
>> >
>> >* util/biosdisk.c (grub_util_biosdisk_get_grub_dev): Issue a warning
>> >when `disk->total_sectors' exceeds ULONG_MAX32 on `pc' partmap.
>> 
>> Why a warning and not an error?  Or perhaps another part of GRUB
>> should return an error?
>
> Uhm.  The person who reported this later told me that the partmap was in fact
> not `pc' but `gpt'.  The problem this patch intended to address may still
> apply, but without reproducing it first, I'd prefer not to run the risk of
> "fixing" something that wasn't broken.

Ok, fine for me.  Please don't forget to add the GRUB_ prefix before
committing :-)

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] fix serial console on LinuxBIOS

[Marco Gerards]

Robert Millan <[EMAIL PROTECTED]> writes:

> On Fri, Nov 09, 2007 at 04:30:44PM +0100, Marco Gerards wrote:
>> Robert Millan <[EMAIL PROTECTED]> writes:
>> 
>> > This patch fixes serial console on LinuxBIOS.
>> >
>> > I'd appreciate comments (specially on the  
>> > addition).
>> >
>> > -- 
>> > Robert Millan
>> >
>> >  I know my rights; I want my phone call!
>> >  What use is a phone call, if you are unable to speak?
>> > (as seen on /.)
>> >
>> 
>> No header ;)
>
> Uhm what header?

That's what I said ;-)

What I meant was something like:

2007-10-31  Robert Millan  <[EMAIL PROTECTED]>

>> >* include/grub/i386/efi/machine.h: New file.
>> >* include/grub/i386/linuxbios/machine.h: Likewise.
>> >* include/grub/i386/pc/machine.h: Likewise.
>> >* include/grub/powerpc/ieee1275/machine.h: Likewise.
>> >* include/grub/sparc64/ieee1275/machine.h: Likewise.
>> >
>> >* term/i386/pc/serial.c: Include .
>> >(serial_hw_io_addr): New variable.
>> >(serial_hw_get_port): Obtain port address from `serial_hw_io_addr'
>> >instead of `(unsigned short *) 0x400'.
>> 
>> This seems fine to me.  What is your intended use for machine.h?  More
>> than just this?
>
> Situations very similar to this one, in that you just need to change a few
> unportable lines while the overall structure of the file remains portable.

What I meant was: what should and shouldn't be added to this .h?

>> Perhaps we can even use autoconf to define this in config.h?  That
>> would be better I think.
>
> autoconf already setups the cpu / machine symlinks.  Why ask it to tell the
> same info twice?

Isn't that what you are doing now?  But I have no objections to this
fix, please commit it :-)

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Avoid crash on empty menu

[Marco Gerards]

Christian Franke <[EMAIL PROTECTED]> writes:

> Marco Gerards wrote:
>>
>>> +  e = get_entry (menu, boot_entry);
>>> +  if (! e)
>>> +   continue; /* menu is empty */
>>>
>>
>> Please use proper interpunctions for comments.
>>
>>
>
> Fixed.
>
> Christian
>
> 2007-11-10  Christian Franke  <[EMAIL PROTECTED]>
>
>   * normal/menu.c (run_menu): Check for empty menu to avoid crash.
>   (grub_menu_run): Likewise.

This looks ok to me.  We can apply this now, it's just a few line.

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] Use getopt_long() instead of argp_parse() in grub-emu

[Christian Franke]

Unlike the other GRUB2 utils, grub-emu uses the glibc extension 
argp_parse(). It is unavailable on Cygwin, which might also be the case 
for other platforms where glibc is not the native runtime.


This patch changes this back to the more traditional getopt_long().

It also fixes the syntax of the path prefix.

Christian

2007-11-10  Christian Franke  <[EMAIL PROTECTED]>

* util/grub-emu.c: Replace argp.h by getopt.h.
(parse_opt): Remove.
(usage): New function.
(main): Replace argp_parse() by getopt_long().
Rename argument variables accordingly.
Add missing "(...)" for root_dev in prefix.


--- grub2.orig/util/grub-emu.c	2007-08-02 19:24:06.0 +0200
+++ grub2/util/grub-emu.c	2007-11-10 16:05:26.84375 +0100
@@ -18,7 +18,7 @@
 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
@@ -90,104 +90,115 @@ grub_machine_fini (void)
 }
 
 
-const char *argp_program_version = PACKAGE_STRING;
-const char *argp_program_bug_address = PACKAGE_BUGREPORT;
-static char doc[] = "GRUB emulator";
-
-static struct argp_option options[] = {
-  {"root-device", 'r', "DEV",  0, "use DEV as the root device [default=guessed]", 0},
-  {"device-map",  'm', "FILE", 0, "use FILE as the device map", 0},
-  {"directory",   'd', "DIR",  0, "use GRUB files in the directory DIR", 0},
-  {"verbose", 'v', 0 , 0, "print verbose messages", 0},
-  {"hold",'H', "SECONDS", OPTION_ARG_OPTIONAL, "wait until a debugger will attach", 0},
-  { 0, 0, 0, 0, 0, 0 }
-};
-
-struct arguments
-{
-  char *root_dev;
-  char *dev_map;
-  char *dir;
-  int hold;
-};
-
-static error_t
-parse_opt (int key, char *arg, struct argp_state *state)
-{
-  struct arguments *args = state->input;
-  
-  switch (key)
-{
-case 'r':
-  args->root_dev = arg;
-  break;
-case 'd':
-  args->dir = arg;
-  break;
-case 'm':
-  args->dev_map = arg;
-  break;
-case 'v':
-  verbosity++;
-  break;
-case 'H':
-  args->hold = arg ? atoi (arg) : -1;
-  break;
-case ARGP_KEY_END:
-  break;
-default:
-  return ARGP_ERR_UNKNOWN;
-}
-  return 0;
+static struct option options[] =
+  {
+{"root-device", required_argument, 0, 'r'},
+{"device-map", required_argument, 0, 'm'},
+{"directory", required_argument, 0, 'd'},
+{"hold", optional_argument, 0, 'H'},
+{"help", no_argument, 0, 'h'},
+{"version", no_argument, 0, 'V'},
+{"verbose", no_argument, 0, 'v'},
+{ 0, 0, 0, 0 }
+  };
+
+static int 
+usage (int status)
+{
+  if (status)
+fprintf (stderr,
+	 "Try ``grub-emu --help'' for more information.\n");
+  else
+printf (
+  "Usage: grub-emu [OPTION]...\n"
+  "\n"
+  "GRUB emulator.\n"
+  "\n"
+  "  -r, --root-device=DEV use DEV as the root device [default=guessed]\n"
+  "  -m, --device-map=FILE use FILE as the device map [default=%s]\n"
+  "  -d, --directory=DIR   use GRUB files in the directory DIR [default=%s]\n"
+  "  -v, --verbose print verbose messages\n"
+  "  -H, --hold[=SECONDS]  wait until a debugger will attach\n"
+  "  -h, --helpdisplay this message and exit\n"
+  "  -V, --version print version information and exit\n"
+  "\n"
+  "Report bugs to <%s>.\n", DEFAULT_DEVICE_MAP, DEFAULT_DIRECTORY, PACKAGE_BUGREPORT);
+  return status;
 }
-
-static struct argp argp = {options, parse_opt, 0, doc, 0, 0, 0};
 
 
 int
 main (int argc, char *argv[])
 {
-  char *dir;
-  
-  struct arguments args =
-{
-  .dir = DEFAULT_DIRECTORY,
-  .dev_map = DEFAULT_DEVICE_MAP,
-  .hold = 0
-};
+  char *root_dev = 0;
+  char *dir = DEFAULT_DIRECTORY;
+  char *dev_map = DEFAULT_DEVICE_MAP;
+  volatile int hold = 0;
+  int opt;
   
   progname = "grub-emu";
-  
-  argp_parse (&argp, argc, argv, 0, 0, &args);
+
+  while ((opt = getopt_long (argc, argv, "r:d:m:vH:hV", options, 0)) != -1)
+switch (opt)
+  {
+  case 'r':
+root_dev = optarg;
+break;
+  case 'd':
+dir = optarg;
+break;
+  case 'm':
+dev_map = optarg;
+break;
+  case 'v':
+verbosity++;
+break;
+  case 'H':
+hold = (optarg ? atoi (optarg) : -1);
+break;
+  case 'h':
+return usage (0);
+  case 'V':
+printf ("%s (%s) %s\n", progname, PACKAGE_NAME, PACKAGE_VERSION);
+return 0;
+  default:
+return usage (1);
+  }
+
+  if (optind < argc)
+{
+  fprintf (stderr, "Unknown extra argument `%s'.\n", argv[optind]);
+  return usage (1);
+}
 
   /* Wait until the ARGS.HOLD variable is cleared by an attached debugger. */
-  if (args.hold && verbosity > 0)
+  if (hold && verbosity > 0)
 printf ("Run \"gdb %s %d\", and set ARGS.HOLD to zero.\n",
 progname, (int) getpid ());
-  while (args.hold)
+  while (hold)
 {
-  if (args.hold > 0)
-

Re: [PATCH] Fix grub-emu curses KEY_* mapping

[Marco Gerards]

Christian Franke <[EMAIL PROTECTED]> writes:

> Marco Gerards wrote:
>
>> Christian Franke <[EMAIL PROTECTED]> writes:
>>
>>> Curses function keys do not work in grub-emu, this patch fixes this in
>>> grub_ncurses_getkey().
>>>
>>
>> Ha!  I once noticed this and forgot to document this bug :(
>>
>>
>>> Another approach would be to remove the scancode translation in
>>> grub_console_getkey(), and check for GRUB_CONSOLE_KEY_* in
>>> grub_cmdline_get() instead. The GRUB_CONSOLE_KEY_* definitions are
>>> already platform specific.
>>>
>>
>> Yes, this is really weird!  All archs define these macros and expect
>> for i386-pc none of them are used.  I actually wonder if any of those
>> work :-)
>>
>>
>
> Even on i386-pc, GRUB_CONSOLE_KEY_* are only used once in the
> translation table of grub_console_getkey(). The defines are not part
> of the console interface definition.
>
> I would suggest to remove the defines from all
> include/grub/PLATFORM/console.h. The GRUB_CONSOLE_KEY_* in
> i386/pc/startup.S can be replaced with the constants itself.

The other definitions have to be used.  I do not like using constant
values in asm/C code, instead I like using macros for this because in
that case its meaning is obvious.

>>> ...
>>>  +  /* XXX: return GRUB_CONSOLE_KEY_* does not work here,
>>> + grub_cmdline_get() does not check for these constants.
>>> + At least on i386-pc, GRUB_CONSOLE_KEY_* are in fact keyboard
>>> + scancodes which are converted into control chars by
>>> + grub_console_getkey(). */
>>> +
>>>
>>
>> I do not think this comment is needed.  You explained it in the
>> email.  I do not like comments that explain why we aren't doing some
>> things, it is usually better to explain why we do something if it is a
>> hack or not obvious.  In this case I prefer no comment.
>>
>>
>>>switch (c)
>>>  {
>>>  case KEY_LEFT:
>>> -  c = GRUB_CONSOLE_KEY_LEFT;
>>> +  c = 2; /*GRUB_CONSOLE_KEY_LEFT*/
>>>break;
>>>
>>
>> Please just remove the comment.  As this appears to be wrong anyways
>> :-)
>>
>>
>
> Done.
>
> Christian
>
> 2007-11-10  Christian Franke  <[EMAIL PROTECTED]>
>
>   * util/console.c (grub_ncurses_getkey): Change curses KEY_* mapping,
>   now return control chars instead of GRUB_CONSOLE_KEY_* constants.
>   This fixes the problem that function keys did not work in grub-emu.

This looks fine to me.  Now we have to wait for the paperwork :-/

--
Marco



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

[patch] Fix for vga terminal

[Vesa Jääskeläinen]

Hi All,

Here is patch for fixing vga terminal. At least it worked fine on qemu.
Now it uses only font system to get fonts so you need to load font
module and use fonts. See gfxterm section on the wiki on how to do that.

I had to edit patch by hand as 64bit patch is still pending and I need
that to compile my codes. But it should still apply nicely to current CVS.

Thanks,
Vesa Jääskeläinen
Index: ChangeLog
===
RCS file: /sources/grub/grub2/ChangeLog,v
retrieving revision 1.438
diff -u -r1.438 ChangeLog
--- ChangeLog   6 Nov 2007 21:23:59 -   1.438
+++ ChangeLog   10 Nov 2007 13:57:07 -
@@ -1,3 +1,23 @@
+2007-11-10  Vesa Jaaskelainen  <[EMAIL PROTECTED]>
+
+   * conf/i386-pc.rmk (pkgdata_MODULES): Added vga.mod.
+   (vga_mod_SOURCES): Added.
+   (vga_mod_CFLAGS): Likewise.
+   (vga_mod_LDFLAGS): Likewise.
+
+   * term/i386/pc/vga.c (get_map_mask): Switch order of arguments in
+   grub_outb() calls.
+   (set_map_mask): Likewise.
+   (set_read_map): Likewise.
+   (set_read_address): Likewise.
+   (vga_font): Removed variable.
+   (get_vga_glyph): Removed function.
+   (invalidate_char): Likewise.
+   (write_char): Changed to use grub_font_get_glyph() for font
+   information.
+   (grub_vga_putchar): Likewise.
+   (grub_vga_getcharwidth): Likewise.
+
 2007-11-06  Robert Millan  <[EMAIL PROTECTED]>
 
* term/i386/pc/serial.c (serial_hw_put): Switch order of arguments
Index: conf/i386-pc.rmk
===
RCS file: /sources/grub/grub2/conf/i386-pc.rmk,v
retrieving revision 1.91
diff -u -r1.91 i386-pc.rmk
--- conf/i386-pc.rmk31 Oct 2007 22:29:20 -  1.91
+++ conf/i386-pc.rmk10 Nov 2007 13:57:13 -
@@ -129,7 +129,8 @@
 pkgdata_MODULES = biosdisk.mod _chain.mod _linux.mod linux.mod normal.mod \
_multiboot.mod chain.mod multiboot.mod reboot.mod halt.mod  \
vbe.mod vbetest.mod vbeinfo.mod video.mod gfxterm.mod \
-   videotest.mod play.mod bitmap.mod tga.mod cpuid.mod serial.mod ata.mod
+   videotest.mod play.mod bitmap.mod tga.mod cpuid.mod serial.mod ata.mod \
+   vga.mod
 
 # For biosdisk.mod.
 biosdisk_mod_SOURCES = disk/i386/pc/biosdisk.c
@@ -251,4 +252,9 @@
 ata_mod_CFLAGS = $(COMMON_CFLAGS)
 ata_mod_LDFLAGS = $(COMMON_LDFLAGS)
 
+# For vga.mod.
+vga_mod_SOURCES = term/i386/pc/vga.c
+vga_mod_CFLAGS = $(COMMON_CFLAGS)
+vga_mod_LDFLAGS = $(COMMON_LDFLAGS)
+
 include $(srcdir)/conf/common.mk
Index: term/i386/pc/vga.c
===
RCS file: /sources/grub/grub2/term/i386/pc/vga.c,v
retrieving revision 1.12
diff -u -r1.12 vga.c
--- term/i386/pc/vga.c  3 Oct 2007 20:13:21 -   1.12
+++ term/i386/pc/vga.c  10 Nov 2007 13:57:15 -
@@ -18,6 +18,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -62,7 +63,6 @@
 static int cursor_state;
 static unsigned char fg_color, bg_color;
 static struct colored_char text_buf[TEXT_WIDTH * TEXT_HEIGHT];
-static unsigned char *vga_font;
 static unsigned char saved_map_mask;
 static int page = 0;
 
@@ -97,11 +97,11 @@
   unsigned char old_data;
   
   old_addr = grub_inb (SEQUENCER_ADDR_PORT);
-  grub_outb (SEQUENCER_ADDR_PORT, MAP_MASK_REGISTER);
+  grub_outb (MAP_MASK_REGISTER, SEQUENCER_ADDR_PORT);
   
   old_data = grub_inb (SEQUENCER_DATA_PORT);
   
-  grub_outb (SEQUENCER_ADDR_PORT, old_addr);
+  grub_outb (old_addr, SEQUENCER_ADDR_PORT);
 
   return old_data;
 }
@@ -113,11 +113,11 @@
   unsigned char old_addr;
   
   old_addr = grub_inb (SEQUENCER_ADDR_PORT);
-  grub_outb (SEQUENCER_ADDR_PORT, MAP_MASK_REGISTER);
+  grub_outb (MAP_MASK_REGISTER, SEQUENCER_ADDR_PORT);
   
-  grub_outb (SEQUENCER_DATA_PORT, mask);
+  grub_outb (mask, SEQUENCER_DATA_PORT);
   
-  grub_outb (SEQUENCER_ADDR_PORT, old_addr);
+  grub_outb (old_addr, SEQUENCER_ADDR_PORT);
 }
 
 /* Set Read Map Register.  */
@@ -128,10 +128,10 @@
   
   old_addr = grub_inb (GRAPHICS_ADDR_PORT);
 
-  grub_outb (GRAPHICS_ADDR_PORT, READ_MAP_REGISTER);
-  grub_outb (GRAPHICS_DATA_PORT, map);
+  grub_outb (READ_MAP_REGISTER, GRAPHICS_ADDR_PORT);
+  grub_outb (map, GRAPHICS_DATA_PORT);
 
-  grub_outb (GRAPHICS_ADDR_PORT, old_addr);
+  grub_outb (old_addr, GRAPHICS_ADDR_PORT);
 }
 
 /* Set start address.  */
@@ -142,19 +142,18 @@
   
   old_addr = grub_inb (CRTC_ADDR_PORT);
   
-  grub_outb (CRTC_ADDR_PORT, START_ADDR_LOW_REGISTER);
-  grub_outb (CRTC_DATA_PORT, start & 0xFF);
+  grub_outb (START_ADDR_LOW_REGISTER, CRTC_ADDR_PORT);
+  grub_outb (start & 0xFF, CRTC_DATA_PORT);
   
-  grub_outb (CRTC_ADDR_PORT, START_ADDR_HIGH_REGISTER);
-  grub_outb (CRTC_DATA_PORT, start >> 8);
+  grub_outb (START_ADDR_HIGH_REGISTER, CRTC_ADDR_PORT);
+  grub_outb (start >> 8, CRTC_DATA_PORT);
 
-  grub_outb (CRTC_ADDR_PORT, old_addr);
+  grub_outb (old_addr, CRTC_ADDR_PORT);
 }
 
 static grub_err_t
 grub_vga_m

Re: [PATCH] Add host open devicename check

[Christian Franke]

Robert Millan wrote:

On Fri, Nov 09, 2007 at 10:17:19PM +0100, Marco Gerards wrote:
  

Robert Millan <[EMAIL PROTECTED]> writes:



On Thu, Oct 25, 2007 at 09:51:38PM +0200, Christian Franke wrote:
  

 static grub_err_t
-grub_host_open (const char *name __attribute((unused)), grub_disk_t disk)
+grub_host_open (const char *name, grub_disk_t disk)
 {
+  if (grub_strcmp(name, "host"))
+  return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "not a host disk");
+


I would test for (disk->id == GRUB_DISK_DEVICE_HOST_ID) instead.  Faster,
and also cleaner/simpler IMHO.
  

It's not possible unfortunately :-(.  This information is about to be
filled in in this same function.



Still seems like an ugly hack to me.  Oh well :-/

  


All disk/* modules' open routines check whether the name (hd%d, ata%d, 
...) is valid and return UNKNOWN_DEVICE on error.
The missing name check in host.c is a bug which can IMO only be fixed 
this way.


Christian



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Add host open devicename check

[Christian Franke]

Marco Gerards wrote:

..

2007-10-25  Christian Franke  <[EMAIL PROTECTED]>

* disk/host.c (grub_host_open): Add check for "host". This fixes
the problem that grub-emu does not find partitions.



Please mention the attribute change.

  

...
 static grub_err_t
-grub_host_open (const char *name __attribute((unused)), grub_disk_t disk)
+grub_host_open (const char *name, grub_disk_t disk)
 {
+  if (grub_strcmp(name, "host"))
+  return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "not a host disk");



Please add a space after the function name and before the "(".

  


Done

Christian

2007-11-10  Christian Franke  <[EMAIL PROTECTED]>

* disk/host.c (grub_host_open): Remove attribute unused from
name parameter. Add check for "host". This fixes the problem
that grub-emu does not find partitions.


--- grub2.orig/disk/host.c	2007-08-02 19:24:05.0 +0200
+++ grub2/disk/host.c	2007-11-10 14:18:11.046875000 +0100
@@ -34,8 +34,11 @@ grub_host_iterate (int (*hook) (const ch
 }
 
 static grub_err_t
-grub_host_open (const char *name __attribute((unused)), grub_disk_t disk)
+grub_host_open (const char *name, grub_disk_t disk)
 {
+  if (grub_strcmp (name, "host"))
+  return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "not a host disk");
+
   disk->total_sectors = 0;
   disk->id = (int) "host";
   
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Fix grub-emu curses KEY_* mapping

[Christian Franke]

Marco Gerards wrote:


Christian Franke <[EMAIL PROTECTED]> writes:
  

Curses function keys do not work in grub-emu, this patch fixes this in
grub_ncurses_getkey().



Ha!  I once noticed this and forgot to document this bug :(

  

Another approach would be to remove the scancode translation in
grub_console_getkey(), and check for GRUB_CONSOLE_KEY_* in
grub_cmdline_get() instead. The GRUB_CONSOLE_KEY_* definitions are
already platform specific.



Yes, this is really weird!  All archs define these macros and expect
for i386-pc none of them are used.  I actually wonder if any of those
work :-)

  


Even on i386-pc, GRUB_CONSOLE_KEY_* are only used once in the 
translation table of grub_console_getkey(). The defines are not part of 
the console interface definition.


I would suggest to remove the defines from all 
include/grub/PLATFORM/console.h. The GRUB_CONSOLE_KEY_* in 
i386/pc/startup.S can be replaced with the constants itself.




...
 
+  /* XXX: return GRUB_CONSOLE_KEY_* does not work here,

+ grub_cmdline_get() does not check for these constants.
+ At least on i386-pc, GRUB_CONSOLE_KEY_* are in fact keyboard
+ scancodes which are converted into control chars by
+ grub_console_getkey(). */
+



I do not think this comment is needed.  You explained it in the
email.  I do not like comments that explain why we aren't doing some
things, it is usually better to explain why we do something if it is a
hack or not obvious.  In this case I prefer no comment.

  

   switch (c)
 {
 case KEY_LEFT:
-  c = GRUB_CONSOLE_KEY_LEFT;
+  c = 2; /*GRUB_CONSOLE_KEY_LEFT*/
   break;



Please just remove the comment.  As this appears to be wrong anyways
:-)

  


Done.

Christian

2007-11-10  Christian Franke  <[EMAIL PROTECTED]>

* util/console.c (grub_ncurses_getkey): Change curses KEY_* mapping,
now return control chars instead of GRUB_CONSOLE_KEY_* constants.
This fixes the problem that function keys did not work in grub-emu.



--- grub2.orig/util/console.c	2007-07-22 01:32:31.0 +0200
+++ grub2/util/console.c	2007-11-10 13:20:02.171875000 +0100
@@ -164,50 +164,50 @@ grub_ncurses_getkey (void)
   switch (c)
 {
 case KEY_LEFT:
-  c = GRUB_CONSOLE_KEY_LEFT;
+  c = 2;
   break;
 
 case KEY_RIGHT:
-  c = GRUB_CONSOLE_KEY_RIGHT;
+  c = 6;
   break;
   
 case KEY_UP:
-  c = GRUB_CONSOLE_KEY_UP;
+  c = 16;
   break;
 
 case KEY_DOWN:
-  c = GRUB_CONSOLE_KEY_DOWN;
+  c = 14;
   break;
 
 case KEY_IC:
-  c = GRUB_CONSOLE_KEY_IC;
+  c = 24;
   break;
 
 case KEY_DC:
-  c = GRUB_CONSOLE_KEY_DC;
+  c = 4;
   break;
 
 case KEY_BACKSPACE:
   /* XXX: For some reason ncurses on xterm does not return
 	 KEY_BACKSPACE.  */
 case 127: 
-  c = GRUB_CONSOLE_KEY_BACKSPACE;
+  c = 8;
   break;
 
 case KEY_HOME:
-  c = GRUB_CONSOLE_KEY_HOME;
+  c = 1;
   break;
 
 case KEY_END:
-  c = GRUB_CONSOLE_KEY_END;
+  c = 5;
   break;
 
 case KEY_NPAGE:
-  c = GRUB_CONSOLE_KEY_NPAGE;
+  c = 3;
   break;
 
 case KEY_PPAGE:
-  c = GRUB_CONSOLE_KEY_PPAGE;
+  c = 7;
   break;
 }
 
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Avoid crash on empty menu

[Christian Franke]

Marco Gerards wrote:



+  e = get_entry (menu, boot_entry);
+  if (! e)
+   continue; /* menu is empty */



Please use proper interpunctions for comments.

  


Fixed.

Christian

2007-11-10  Christian Franke  <[EMAIL PROTECTED]>

* normal/menu.c (run_menu): Check for empty menu to avoid crash.
(grub_menu_run): Likewise.



--- grub2.orig/normal/menu.c	2007-08-20 16:35:20.0 +0200
+++ grub2/normal/menu.c	2007-11-10 12:59:54.953125000 +0100
@@ -412,7 +412,11 @@ run_menu (grub_menu_t menu, int nested)
 	  goto refresh;
 
 	case 'e':
-	  grub_menu_entry_run (get_entry (menu, first + offset));
+		{
+		  grub_menu_entry_t e = get_entry (menu, first + offset);
+		  if (e)
+		grub_menu_entry_run (e);
+		}
 	  goto refresh;
 	  
 	default:
@@ -451,10 +455,13 @@ grub_menu_run (grub_menu_t menu, int nes
   if (boot_entry < 0)
 	break;
 
+  e = get_entry (menu, boot_entry);
+  if (! e)
+	continue; /* Menu is empty.  */
+	
   grub_cls ();
   grub_setcursor (1);
 
-  e = get_entry (menu, boot_entry);
   grub_printf ("  Booting \'%s\'\n\n", e->title);
   
   run_menu_entry (e);
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] Building on Ubuntu 7.10 x86-64

[Vesa Jääskeläinen]

Hi All,

I needed to do following modification in order to build grub2 on my
64bit laptop. In case there is no complaints I will commit it to CVS.

Additionally I needed to install gcc-multilib package (and normal
packages like ruby, lzo...).

And after that just doing following made runnable binaries (at least in
qemu):

./autogen.sh

mkdir build-x86_64
cd build-x86_64

../configure --with-platform=pc --target=x86_64

make

Thanks,
Vesa Jääskeläinen
Index: ChangeLog
===
RCS file: /sources/grub/grub2/ChangeLog,v
retrieving revision 1.438
diff -u -r1.438 ChangeLog
--- ChangeLog   6 Nov 2007 21:23:59 -   1.438
+++ ChangeLog   10 Nov 2007 11:22:13 -
@@ -1,3 +1,11 @@
+2007-11-10  Vesa Jaaskelainen  <[EMAIL PROTECTED]>
+
+   * conf/i386-pc.rmk (boot_img_LDFLAGS): Use COMMON_FLAGS for target
+   flags.
+   (pxeboot_img_LDFLAGS): Likewise.
+   (diskboot_img_LDFLAGS): Likewise.
+   (kernel_img_LDFLAGS): Likewise.
+
 2007-11-06  Robert Millan  <[EMAIL PROTECTED]>
 
* term/i386/pc/serial.c (serial_hw_put): Switch order of arguments
Index: conf/i386-pc.rmk
===
RCS file: /sources/grub/grub2/conf/i386-pc.rmk,v
retrieving revision 1.91
diff -u -r1.91 i386-pc.rmk
--- conf/i386-pc.rmk31 Oct 2007 22:29:20 -  1.91
+++ conf/i386-pc.rmk10 Nov 2007 11:22:19 -
@@ -10,17 +10,17 @@
 # For boot.img.
 boot_img_SOURCES = boot/i386/pc/boot.S
 boot_img_ASFLAGS = $(COMMON_ASFLAGS)
-boot_img_LDFLAGS = -nostdlib -Wl,-N,-Ttext,7C00
+boot_img_LDFLAGS = $(COMMON_LDFLAGS) -Wl,-N,-Ttext,7C00
 
 # For pxeboot.img
 pxeboot_img_SOURCES = boot/i386/pc/pxeboot.S
 pxeboot_img_ASFLAGS = $(COMMON_ASFLAGS)
-pxeboot_img_LDFLAGS = -nostdlib -Wl,-N,-Ttext,7C00
+pxeboot_img_LDFLAGS = $(COMMON_LDFLAGS) -Wl,-N,-Ttext,7C00
 
 # For diskboot.img.
 diskboot_img_SOURCES = boot/i386/pc/diskboot.S
 diskboot_img_ASFLAGS = $(COMMON_ASFLAGS)
-diskboot_img_LDFLAGS = -nostdlib -Wl,-N,-Ttext,8000
+diskboot_img_LDFLAGS = $(COMMON_LDFLAGS) -Wl,-N,-Ttext,8000
 
 # For kernel.img.
 kernel_img_SOURCES = kern/i386/pc/startup.S kern/main.c kern/device.c \
@@ -37,7 +37,7 @@
machine/memory.h machine/loader.h machine/vga.h machine/vbe.h
 kernel_img_CFLAGS = $(COMMON_CFLAGS)
 kernel_img_ASFLAGS = $(COMMON_ASFLAGS)
-kernel_img_LDFLAGS = -nostdlib -Wl,-N,-Ttext,8200 $(COMMON_CFLAGS)
+kernel_img_LDFLAGS = $(COMMON_LDFLAGS) -Wl,-N,-Ttext,8200 $(COMMON_CFLAGS)
 
 MOSTLYCLEANFILES += symlist.c kernel_syms.lst
 DEFSYMFILES += kernel_syms.lst
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel