Re: [PATCH] PXE support for grub2

[Bean]

On Tue, Aug 5, 2008 at 11:15 PM, Bean <[EMAIL PROTECTED]> wrote:
> Fixed and committed.

Fixed a few bugs, also add some doc in the wiki for pxe boot:

http://grub.enbug.org/PXEBOOT

-- 
Bean


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

Re: `initrd' grub2's mods or not

[Bean]

On Wed, Aug 6, 2008 at 12:19 PM, Albert <[EMAIL PROTECTED]> wrote:
> hi all,
>
> i have an idea about loading grub2' file (.mod, fonts, images) from
> disk: since core.img is the kernel why not make related stuffs into a
> 'initrd' file? the reason is, so many files will be loaded from media, we
> may need more time to do this from a cd/dvd.
>
> is there any possible?

Hi,

This function is already in grub2, it's called memdisk. The basic idea
is that you can pack all the files you want into a archive file such
as cpio or tar, then embed it in core.img. After startup, the files
can be accessed with the (memdisk) device.

-- 
Bean


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

`initrd' grub2's mods or not

[Albert]

hi all,

i have an idea about loading grub2' file (.mod, fonts, images) from disk: 
since core.img is the kernel why not make related stuffs into a 'initrd' file? 
the reason is, so many files will be loaded from media, we may need more time 
to do this from a cd/dvd.

is there any possible?
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Remove conf/*.mk from svn

[Viswesh S]

Hi,
I have a small suggestion.
Why cant we document in a small file, the procedures, or the dont's while 
adding modules or modifying grub2.A FAQ file may be.
That will help us in the long run.
Viswesh


- Original Message 
From: Felix Zielcke <[EMAIL PROTECTED]>
To: The development of GRUB 2 
Sent: Tuesday, 5 August, 2008 9:25:43 PM
Subject: Re: Remove conf/*.mk from svn

Am Dienstag, den 05.08.2008, 17:48 +0200 schrieb Marco Gerards:

> 
> Actually, since ruby is required to generate these files, I guess we
> can better keep the .mk files.
> 

Just a little note from me:

The Debian package build depends already on ruby so this won't be a
problem for all Debian guys and especially not for Robert or me.

On Debian/Ubuntu you can just use 'apt-get build-dep grub2' and it
installs every package for you which you need to build grub2 so ruby
will be always needed/installed for the Debian package.

But well Debian isn't the only distribution and I don't know how others
deal with grub2 :)



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



  Download prohibited? No problem. CHAT from any browser, without download. 
Go to http://in.webmessenger.yahoo.com/___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] power management on coreboot

[Robert Millan]

Committed.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."


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

[PATCH] use grub_get_time_ms() instead of grub_get_rtc()

[Robert Millan]

This replaces usage of grub_get_rtc() with grub_get_time_ms() in a few places
throurough the code.  Aside from the gained precision, it also permits that
functionality to work on Coreboot.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."
2008-08-06  Robert Millan  <[EMAIL PROTECTED]>

	* kern/disk.c: Replace `' with `'.
	(grub_last_time): Change type to grub_uint64_t.
	(grub_disk_open): Migrate code from to using grub_get_time_ms().
	(grub_disk_close): Likewise.

	* normal/menu.c: Replace `' with `'.
	(run_menu): Migrate code from to using grub_get_time_ms().

	* util/misc.c (grub_get_time_ms): New function.

Index: kern/disk.c
===
--- kern/disk.c	(revision 1780)
+++ kern/disk.c	(working copy)
@@ -22,13 +22,13 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 
 #define	GRUB_CACHE_TIMEOUT	2
 
 /* The last time the disk was used.  */
-static unsigned long grub_last_time = 0;
+static grub_uint64_t grub_last_time = 0;
 
 
 /* Disk cache.  */
@@ -215,7 +215,7 @@
   grub_disk_t disk;
   grub_disk_dev_t dev;
   char *raw = (char *) name;
-  unsigned long current_time;
+  grub_uint64_t current_time;
 
   grub_dprintf ("disk", "Opening `%s'...\n", name);
 
@@ -280,10 +280,10 @@
 
   /* The cache will be invalidated about 2 seconds after a device was
  closed.  */
-  current_time = grub_get_rtc ();
+  current_time = grub_get_time_ms ();
 
   if (current_time > (grub_last_time
-		  + GRUB_CACHE_TIMEOUT * GRUB_TICKS_PER_SECOND))
+		  + GRUB_CACHE_TIMEOUT * 1000))
 grub_disk_cache_invalidate_all ();
   
   grub_last_time = current_time;
@@ -315,7 +315,7 @@
 (disk->dev->close) (disk);
 
   /* Reset the timer.  */
-  grub_last_time = grub_get_rtc ();
+  grub_last_time = grub_get_time_ms ();
 
   grub_free (disk->partition);
   grub_free ((void *) disk->name);
Index: normal/menu.c
===
--- normal/menu.c	(revision 1780)
+++ normal/menu.c	(working copy)
@@ -21,7 +21,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 
@@ -326,7 +326,7 @@
 run_menu (grub_menu_t menu, int nested)
 {
   int first, offset;
-  unsigned long saved_time;
+  grub_uint64_t saved_time;
   int default_entry;
   int timeout;
   
@@ -351,7 +351,7 @@
 }
 
   /* Initialize the time.  */
-  saved_time = grub_get_rtc ();
+  saved_time = grub_get_time_ms ();
 
  refresh:
   grub_setcursor (0);
@@ -371,10 +371,10 @@
   
   if (timeout > 0)
 	{
-	  unsigned long current_time;
+	  grub_uint64_t current_time;
 
-	  current_time = grub_get_rtc ();
-	  if (current_time - saved_time >= GRUB_TICKS_PER_SECOND)
+	  current_time = grub_get_time_ms ();
+	  if (current_time - saved_time >= 1000)
 	{
 	  timeout--;
 	  set_timeout (timeout);
Index: util/misc.c
===
--- util/misc.c	(revision 1780)
+++ util/misc.c	(working copy)
@@ -33,6 +33,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 /* Include malloc.h, only if memalign is available. It is known that
@@ -284,6 +285,16 @@
 	 * GRUB_TICKS_PER_SECOND / 100));
 }
 
+grub_uint64_t
+grub_get_time_ms (void)
+{
+  struct timeval tv;
+
+  gettimeofday (&tv, 0);
+  
+  return (tv.tv_sec * 1000 + tv.tv_usec / 1000);
+}
+
 void 
 grub_arch_sync_caches (void *address __attribute__ ((unused)),
 		   grub_size_t len __attribute__ ((unused)))
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

grub-ieee1275 on powerpc says "DEFAULT CATCH!" and then drops me into Open Firmware

[Daniel Kahn Gillmor]

Hey folks--

I'm trying to create a grub-based rescue USB stick for PowerPC
machines.

I've successfully gotten one machine to boot to this device, and grub
can see both the USB disk itself and the onboard (ide) HD.

However, when i try to load a linux kernel from either disk, i get an
error:

grub> linux (hd)/boot/vmlinux-2.6.25-2-powerpc
DEFAULT CATCH!, code=300 at %SRR0: 00013ae4 %SRR1: 3030

(this is from my scribbled notes, not from a serial console,
unfortunately).

at this point, i get dropped to openfirmware, but it's in a
sufficiently weird state that even "mac-boot" doesn't work for me.  If
i reboot the machine, i can boot normally (via ofboot/yaboot
combination off of onboard disk).

This is with 1.96+20080724-5 and the kernel from debian lenny.

The machine in question is an old iBook (clamshell model) with 96M of
RAM, described by lshw as:

product: iBook (first generation)
vendor: Copyright 1983-1999 Apple Computer, Inc. All Rights Reserved

Any thoughts about what i might need to do to get grub to boot a linux
kernel properly?  Should i try to load something different from grub?

Regards,

--dkg

0 clam:~# cat /proc/cpuinfo 
processor   : 0
cpu : 740/750
temperature : 54-56 C (uncalibrated)
clock   : 299.97MHz
revision: 131.0 (pvr 0008 8300)
bogomips: 33.15
timebase: 16644650
platform: PowerMac
machine : PowerBook2,1
motherboard : PowerBook2,1 MacRISC Power Macintosh
detected as : 64 (iBook (first generation))
pmac flags  : 000d
L2 cache: 512K unified
pmac-generation : NewWorld
0 clam:~# 


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

Re: [PATCH][RFC] gdb support in GRUB build system

[Robert Millan]

On Tue, Aug 05, 2008 at 01:50:21PM -0700, Colin D Bennett wrote:
> On Tue, 5 Aug 2008 22:14:03 +0200
> Robert Millan <[EMAIL PROTECTED]> wrote:
> 
> > On Tue, Aug 05, 2008 at 07:42:29AM -0700, Colin D Bennett wrote:
> > > 
> > > We could make it a 'configure' option like '--enable-debug' or
> > > something, but it seems like minimal overhead to generate the .elf
> > > files for gdb in addition to "real" GRUB files.
> > 
> > I'd favour the --enable-debug.  Even if it's a small overhead, it
> > becomes a big problem when you are out of space and need to start
> > looking around to see what can be shaved off.
> 
> In that case, should --enable-debug be the default, or --disable-debug?

I'd prefer to disable debug as default.  What do others think?

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."


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

Re: RFC: conf/i386.rmk

[Pavel Roskin]

On Tue, 2008-08-05 at 22:07 +0200, Robert Millan wrote:
> On Tue, Aug 05, 2008 at 12:49:46PM -0400, Pavel Roskin wrote:
> > On Tue, 2008-08-05 at 09:58 +0300, Vesa Jääskeläinen wrote:
> > 
> > > > I tried moving more stuff to common.rmk many times but gave up every
> > > > time.  One of the reasons is that the sparc64 support is very
> > > > out-of-date and doesn't use common.rmk at all.  I cannot even test it
> > > > (well, I haven't tries hard).
> > > 
> > > If sparc64 support is out of date and the maintainer is nowhere to be 
> > > seen then I think you can put that support to graveyard until someone 
> > > comes up to update it. Putting all common stuff to every platform to 
> > > common.rmk is a good way and the only way (in my opinion) to go forward.
> > 
> > OK, then there is an issue that it doesn't work if done naively.
> 
> What is the problem exactly?

Take all text in conf/i386-pc.rmk from "For grub-emu" to "Scripts" and
move it to the end of conf/common.rmk.  Run autogen.sh, configure
--enable-grub-emu and make.  You'll get:

make: *** No rule to make target `grub-emu', needed by `all-local'.
Stop.

-- 
Regards,
Pavel Roskin


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

Re: [PATCH][RFC] gdb support in GRUB build system

[Colin D Bennett]

On Tue, 5 Aug 2008 22:14:03 +0200
Robert Millan <[EMAIL PROTECTED]> wrote:

> On Tue, Aug 05, 2008 at 07:42:29AM -0700, Colin D Bennett wrote:
> > 
> > We could make it a 'configure' option like '--enable-debug' or
> > something, but it seems like minimal overhead to generate the .elf
> > files for gdb in addition to "real" GRUB files.
> 
> I'd favour the --enable-debug.  Even if it's a small overhead, it
> becomes a big problem when you are out of space and need to start
> looking around to see what can be shaved off.

In that case, should --enable-debug be the default, or --disable-debug?

Colin


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

Seeking for the start

[administrador02032]

Hi:

I am new. You know. I have around two months studying grub. But I feel 
myself lost. I understand the structure and the language. But I do not
have a start ( except boot's module ). What I want to know in general
terms is :

General characteristics of the kernel. (It was maded in order to do what?)
General characteristics out of every module.
I want knowing if exists some engineering design associated with grub and
his development.

I hope not to seem so lost as actually I seat  myself.

Thanks

Alex




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

Re: [PATCH] High resolution time/TSC patch v3

[Robert Millan]

On Tue, Aug 05, 2008 at 01:59:28PM +0200, Marco Gerards wrote:
> Hi Colin,
> 
> Colin D Bennett <[EMAIL PROTECTED]> writes:
> 
> > Another updated TSC patch.  Now it detects TSC support for x86 CPUs at
> > runtime and selects either the TSC or RTC time source.  This way 386
> > and 486 CPUs without RDTSC instruction support are supported.
> >
> > Robert Millan was interested in getting this patch merged for the
> > Coreboot port, so I decided to take another crack at it.
> >
> > Comments are welcome!
> 
> I have just committed your patch, with the ChangeLog changes I
> proposed.  Please have a look to see what I changed.

Ok I committed my followup patch too.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."


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

Re: [PATCH][RFC] gdb support in GRUB build system

[Robert Millan]

On Tue, Aug 05, 2008 at 07:42:29AM -0700, Colin D Bennett wrote:
> 
> We could make it a 'configure' option like '--enable-debug' or
> something, but it seems like minimal overhead to generate the .elf
> files for gdb in addition to "real" GRUB files.

I'd favour the --enable-debug.  Even if it's a small overhead, it becomes a
big problem when you are out of space and need to start looking around to see
what can be shaved off.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."


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

Re: [PATCH] Misc patches for grub2

[Robert Millan]

On Tue, Aug 05, 2008 at 12:36:06PM +0200, Marco Gerards wrote:
> >
> > The name appleloader may be a little confusing, bootcamp seems to be a
> > better choice.
> 
> How about legacyloader or even legacy?

Legacy is ambigous.  Does it mean BIOS?  msdos partmap?  Or perhaps EFI?

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."


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

Re: [PATCH] update-grub for Cygwin

[Robert Millan]

On Tue, Aug 05, 2008 at 02:13:43PM +0200, Christian Franke wrote:
> >
> >Why not have the user write a custom entry then?  I think it clutters the
> >user interface to add options for everything.  If a corner case (boot a non
> >native disk, can't use os-prober) can be supported by creating a new config
> >file, why not do that?  It was the whole reason for designing update-grub 
> >to
> >be easily extensible.
> 
> Even with the new configuration parameter, user can still decide to 
> leave the parameter unset and write a custom entry.

My concern is not with presenting too many options to the user, but with having
to support them.  In general, if a use case can be supported without adding a
new user option, I think it's much better not to add it.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."


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

Re: RFC: conf/i386.rmk

[Robert Millan]

On Tue, Aug 05, 2008 at 12:49:46PM -0400, Pavel Roskin wrote:
> On Tue, 2008-08-05 at 09:58 +0300, Vesa Jääskeläinen wrote:
> 
> > > I tried moving more stuff to common.rmk many times but gave up every
> > > time.  One of the reasons is that the sparc64 support is very
> > > out-of-date and doesn't use common.rmk at all.  I cannot even test it
> > > (well, I haven't tries hard).
> > 
> > If sparc64 support is out of date and the maintainer is nowhere to be 
> > seen then I think you can put that support to graveyard until someone 
> > comes up to update it. Putting all common stuff to every platform to 
> > common.rmk is a good way and the only way (in my opinion) to go forward.
> 
> OK, then there is an issue that it doesn't work if done naively.

What is the problem exactly?

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."


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

Re: Remove conf/*.mk from svn

[Colin D Bennett]

On Tue, 05 Aug 2008 20:41:53 +0200
Javier Martín <[EMAIL PROTECTED]> wrote:

> El mar, 05-08-2008 a las 17:48 +0200, Marco Gerards escribió:
> > Vesa Jääskeläinen <[EMAIL PROTECTED]> writes:
> > 
> > > Marco Gerards wrote:
> > >> Colin D Bennett <[EMAIL PROTECTED]> writes:
> > >>> I think we should remove conf/*.mk from the Subversion
> > >>> repository.  If people are going to be developing on GRUB and
> > >>> checking out svn branches, then I think it's fine to require
> > >>> them to have Ruby.  For released tarballs that we expect
> > >>> non-developers to use, we just need to generate the *.mk files
> > >>> and include them in the tarball.
> > >>
> > >> I do not have problems with this.  Besides this, it will stop
> > >> people from sending in patches with .mk changes in it :-)
> > >
> > > I think Okuji's objection is based on fact that it makes it
> > > harder for people to compile from sources. Now what if we would
> > > generate those files when making a release? Of course this should
> > > be enabled to script/makefile to make it automatically so it is
> > > not forgotten ;)
> > 
> > Right.  Just to be clear, personally I didn't have these objections
> > but Okuji has.
> > 
> > Actually, since ruby is required to generate these files, I guess we
> > can better keep the .mk files.
> Why not rewrite genmk.rb in a more common language (i.e. with an
> interpreter more commonly found in stock GNU installs) like Python or
> Perl?

Fine with me.  It shouldn't be too hard for someone who understands it.

Based on the discussions following my initial suggestion, it sounds
like it is considered too much work for people compiling from a svn
checkout to install Ruby?

Did I emphasize enough that released tarballs or any sort of archived
snapshot should be generated *with* the .mk files?  I am concerned only
with the files under version control -- the point being that files
generated from often-modified files (such as the ``conf/*.rmk`` which
are often modified) have no business being under version control.  I
think that this is even more important than lesser modified files such
as the configure script, etc., since fewer developers have to touch
that.

Regards,
Colin


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

Re: Remove conf/*.mk from svn

[Javier Martín]

El mar, 05-08-2008 a las 17:48 +0200, Marco Gerards escribió:
> Vesa Jääskeläinen <[EMAIL PROTECTED]> writes:
> 
> > Marco Gerards wrote:
> >> Colin D Bennett <[EMAIL PROTECTED]> writes:
> >>> I think we should remove conf/*.mk from the Subversion repository.  If
> >>> people are going to be developing on GRUB and checking out svn
> >>> branches, then I think it's fine to require them to have Ruby.  For
> >>> released tarballs that we expect non-developers to use, we just need to
> >>> generate the *.mk files and include them in the tarball.
> >>
> >> I do not have problems with this.  Besides this, it will stop people
> >> from sending in patches with .mk changes in it :-)
> >
> > I think Okuji's objection is based on fact that it makes it harder for
> > people to compile from sources. Now what if we would generate those
> > files when making a release? Of course this should be enabled to
> > script/makefile to make it automatically so it is not forgotten ;)
> 
> Right.  Just to be clear, personally I didn't have these objections
> but Okuji has.
> 
> Actually, since ruby is required to generate these files, I guess we
> can better keep the .mk files.
Why not rewrite genmk.rb in a more common language (i.e. with an
interpreter more commonly found in stock GNU installs) like Python or
Perl?

Habbit


signature.asc
Description: Esta parte del mensaje está firmada	digitalmente
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Misc patches for grub2

[Colin D Bennett]

On Tue, 5 Aug 2008 22:14:37 +0800
Bean <[EMAIL PROTECTED]> wrote:

> On Tue, Aug 5, 2008 at 6:36 PM, Marco Gerards <[EMAIL PROTECTED]>
> wrote:
> > For some reason I have some doubts about lib/.  But I do not have a
> > better name in mind either.  What does belong in there?  Do you
> > happen to have other names in mind as a suggestion? :-)
> 
> The files in this directory are used by both modules and utilities,
> while files in util/ are only used by utilities.
> 
> I also sense that lib is a little strange, but I can't think of a
> better name. Some alternative name could be: shared, common, helper.

Until recently, it looked like only the LZMA code was in 'lib/'.  This
led me to think along the following lines:  Since the lzma code is in
lib, which is essentially just a modified import of the LZMA SDK 4.58
beta, I thought that lib's intent was something like:

  Code imported from other projects.
  Although this code may have been modified from its upstream form to
  work for GRUB, it is still, in essence, a library that is maintained
  separately from GRUB, and we should be able to stay in sync with the
  upstream project by integrating new versions when we want to.

By keeping imported code as close to its original form as possible
(and in a logical, self contained source tree location such as
'lib/lzma/'), it will make it much easier to upgrade that imported
code to a new version of the upstream.  Perhaps when LZMA SDK 4.58
final is released they will have fixed a bug or two... then it's easy
to bring that forward if we haven't changed the code too much in GRUB.

Just a thought on what 'lib/' *could* be...

Regards,
Colin


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

Re: grub-probe detects ext4 wronly as ext2

[Felix Zielcke]

Am Freitag, den 04.07.2008, 03:20 +0200 schrieb Javier Martín:

> That was it. I will post no more in this thread. Do whatever you please
> with the patch - I'll just request some more people from the GRUB dev
> team to review the thing, instead of the tennis match we've had here
> (and I appreciate all matches, even the ones I lose).

I'd like to bring this topic now up again and yes I know this isn't the
last message about it :)

My Bugreport on Debian BTS about this is still open just retitele'd [0]

I really would like to have ext2.mod reject unknown INCOMPAT flags
instead of just failing to boot.
It happened to me again that I forgot that I hadn't updated grub2 on a
VM where I still prefer ext4 and with playing with grub-legacy I failed
to reming that it can't work aynway :)

Thanks again to Bean that for me ext4 still works fine, but for the
future I'd like to have grub-install failing instead of real grub on the
next (re)boot.

And thanks again to Robert, telling me in the report to message
grub-devel about it, it was the first step joining you :)

[0] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=488235



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

Re: [PATCH] Drivemap module

[Colin D Bennett]

On Tue, 05 Aug 2008 13:23:11 +0200
Marco Gerards <[EMAIL PROTECTED]> wrote:

> Hi Javier,
> 
> Javier Martín <[EMAIL PROTECTED]> writes:
> > While tools like `svn diff' can help in these kind of problems,
> > their utility is greatly diminished if the offending change is part
> > of a multi-line change. Besides, sometimes checks like "if (a=b)",
> > or more frequently "if (a=f())" are intentionally used in C, so the
> > error might become even more difficult to spot and correct. I ask
> > for a deep reflection on this issue: maybe I'm dead wrong and an
> > arrogant brat in my attempt to tackle the Holy GNU Coding
> > Standards, but I'd like to ask input from more people on this.
> 
> I will refuse patches with "if (a = f())", if that makes you sleep
> better ;-)

Since that will generate a gcc warning, I hope that the author would at
least do

  if ((a = f ()))

to indicate to gcc that you mean what you said.

Regards,
Colin


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

Re: [PATCH] Drivemap module

[Colin D Bennett]

On Tue, 05 Aug 2008 01:10:25 +0200
Javier Martín <[EMAIL PROTECTED]> wrote:

> Besides, sometimes checks like "if (a=b)", or more
> frequently "if (a=f())" are intentionally used in C, so the error
> might become even more difficult to spot and correct.

Respect and heed the gcc warnings!!

Note that gcc will warn you (you *are* using -Wall, at least, right?)
if you try to do

  if (a = b)

but if you really mean it, you should surround the expression with an
extra pair of parentheses:

  if ((a = b))

Use of the value of an assignment in an expression is common in code
like

  if ((n = read (fd, buf, sz)) > 0)
/* ... use the data in buf ... */

Here's an example showing gcc being helpful:

  $ cat a.c
  int f()
  {
  int a = 1;
  int b;
  if ((b = a))
return 1;
  return 0;
  }
  $ cat b.c
  int f()
  {
  int a = 1;
  int b;
  if (b = a)
return 1;
  return 0;
  }
  $ gcc -Wall -c a.c
  $ gcc -Wall -c b.c
  b.c: In function ‘f’:
  b.c:5: warning: suggest parentheses around assignment used as truth
  $ 

> New version of the patch follows. I must say that the discussion here
> has become pretty interesting without degrading into flamewarring -
> who whats to call Mr. Torvalds and Prof. Tanembaum? ^^

Well, it sounds to me like you are trying your best to set Marco ablaze,
but he is delightfully fire-retardant.

Regards,
Colin


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

Re: RFC: conf/i386.rmk

[Pavel Roskin]

On Tue, 2008-08-05 at 09:58 +0300, Vesa Jääskeläinen wrote:

> > I tried moving more stuff to common.rmk many times but gave up every
> > time.  One of the reasons is that the sparc64 support is very
> > out-of-date and doesn't use common.rmk at all.  I cannot even test it
> > (well, I haven't tries hard).
> 
> If sparc64 support is out of date and the maintainer is nowhere to be 
> seen then I think you can put that support to graveyard until someone 
> comes up to update it. Putting all common stuff to every platform to 
> common.rmk is a good way and the only way (in my opinion) to go forward.

OK, then there is an issue that it doesn't work if done naively.  Either
genmk.rb should sort statements or we need some makefile magic
(recursively expanded variables, perhaps) that would be transparent to
genmk.rb.  Maybe we need both.

We may want to have another common file for all ieee1275 pratforms.

-- 
Regards,
Pavel Roskin


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

Re: Remove conf/*.mk from svn

[Felix Zielcke]

Am Dienstag, den 05.08.2008, 17:48 +0200 schrieb Marco Gerards:

> 
> Actually, since ruby is required to generate these files, I guess we
> can better keep the .mk files.
> 

Just a little note from me:

The Debian package build depends already on ruby so this won't be a
problem for all Debian guys and especially not for Robert or me.

On Debian/Ubuntu you can just use 'apt-get build-dep grub2' and it
installs every package for you which you need to build grub2 so ruby
will be always needed/installed for the Debian package.

But well Debian isn't the only distribution and I don't know how others
deal with grub2 :)



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

Re: Remove conf/*.mk from svn

[Marco Gerards]

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

> Marco Gerards wrote:
>> Colin D Bennett <[EMAIL PROTECTED]> writes:
>>> I think we should remove conf/*.mk from the Subversion repository.  If
>>> people are going to be developing on GRUB and checking out svn
>>> branches, then I think it's fine to require them to have Ruby.  For
>>> released tarballs that we expect non-developers to use, we just need to
>>> generate the *.mk files and include them in the tarball.
>>
>> I do not have problems with this.  Besides this, it will stop people
>> from sending in patches with .mk changes in it :-)
>
> I think Okuji's objection is based on fact that it makes it harder for
> people to compile from sources. Now what if we would generate those
> files when making a release? Of course this should be enabled to
> script/makefile to make it automatically so it is not forgotten ;)

Right.  Just to be clear, personally I didn't have these objections
but Okuji has.

Actually, since ruby is required to generate these files, I guess we
can better keep the .mk files.

--
Marco



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

Re: Remove conf/*.mk from svn

[Vesa Jääskeläinen]

Marco Gerards wrote:

Colin D Bennett <[EMAIL PROTECTED]> writes:

I think we should remove conf/*.mk from the Subversion repository.  If
people are going to be developing on GRUB and checking out svn
branches, then I think it's fine to require them to have Ruby.  For
released tarballs that we expect non-developers to use, we just need to
generate the *.mk files and include them in the tarball.


I do not have problems with this.  Besides this, it will stop people
from sending in patches with .mk changes in it :-)


I think Okuji's objection is based on fact that it makes it harder for 
people to compile from sources. Now what if we would generate those 
files when making a release? Of course this should be enabled to 
script/makefile to make it automatically so it is not forgotten ;)


Thanks,
Vesa Jääskeläinen


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

Re: [PATCH] skip over invalid BSD partitions

[Marco Gerards]

Felix Zielcke <[EMAIL PROTECTED]> writes:

> Am Dienstag, den 05.08.2008, 15:10 +0200 schrieb Marco Gerards:
>> Felix Zielcke <[EMAIL PROTECTED]> writes:
>> 
>> > 2008-08-05  Felix Zielcke  <[EMAIL PROTECTED]>
>> >
>> >* partmap/pc.c (pc_partition_map_iterate): Do not abort on an invalid 
>> > BSD
>> >magic or if there's no space left, use grub_dprintf to issue a warning.
>> 
>> Looks fine.
> Hurray.
>
>> > +  {
>> > +grub_dprintf ("partition",
>> > +  "invalid disk label magic 0x%x on partition 
>> > %d\n"
>> > +  label.magic, p.index);
>> > +continue;
>> > +  }
>> 
>> Isn't a comma missing after the second string?  I am surprised that
>> this compiles.  Or am I missing something?
>> 
> You're right, shame on me I notice to missing spaces but not a missing
> comma.
> Attached.

Seems fine to me :-)

--
Marco



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

Re: [PATCH][RFC] gdb support in GRUB build system

[Felix Zielcke]

Am Dienstag, den 05.08.2008, 08:11 -0700 schrieb Colin D Bennett:
> 
> Interesting.  Well, I think I used the 'load_all_modules' function
> instead of just 'load_modules'.  And IIRC, I first used the GRUB legacy
> floppy image that was provided to chainload GRUB 2 over QEMU's built-in
> tftp, but I think that directly booting GRUB 2 in QEMU for debugging
> did not work... I'll have to try it again to verify that.
> 

The gdb-macros patch on his fedora page[0] has only load_modules where
the help text of it says it loads all symbols.
I didn't try the chainload grub2 with tftp way.
I have used directly a grub2 floppy img for that and it did work for the
functions not in a module.
I didn't make anything special, just recompiled grub2 with that page, we
both updated, and CFLAGS="-O0 -g3"
and then created the grub2_floppy.img

[0] http://fedorapeople.org/~lkundrak/grub2/




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

Re: [PATCH] PXE support for grub2

[Bean]

On Tue, Aug 5, 2008 at 4:16 PM, Marco Gerards <[EMAIL PROTECTED]> wrote:
> Bean <[EMAIL PROTECTED]> writes:
>
 +GRUB_MOD_INIT(pxe)
 +{
 +  (void) mod;/* To stop warning. */
 +
 +  grub_pxe_detect ();
 +  if (grub_pxe_pxenv)
 +{
 +  grub_disk_dev_register (&grub_pxe_dev);
 +  grub_fs_register (&grub_pxefs_fs);
>>>
>>> filesystems belong in fs/
>>
>> Perhaps I should place it in fs/i386/pc ?
>
> I think it would make things clearer, although I am not too sure :-)
>
 +struct grub_pxenv
 +{
 +  grub_uint8_t signature[6]; /* 'PXENV+' */
 +  grub_uint16_t version; /* MSB = major, LSB = minor */
 +  grub_uint8_t length;   /* structure length */
 +  grub_uint8_t checksum; /* checksum pad */
 +  grub_uint32_t rm_entry;/* SEG:OFF to PXE entry point */
 +  grub_uint32_t  pm_offset;  /* Protected mode entry */
 +  grub_uint16_t pm_selector; /* Protected mode selector */
 +  grub_uint16_t stack_seg;   /* Stack segment address */
 +  grub_uint16_t  stack_size; /* Stack segment size (bytes) */
 +  grub_uint16_t bc_code_seg; /* BC Code segment address */
 +  grub_uint16_t  bc_code_size;   /* BC Code segment size (bytes) */
 +  grub_uint16_t  bc_data_seg;/* BC Data segment address */
 +  grub_uint16_t  bc_data_size;   /* BC Data segment size (bytes) */
 +  grub_uint16_t  undi_data_seg;  /* UNDI Data segment address */
 +  grub_uint16_t  undi_data_size; /* UNDI Data segment size (bytes) */
 +  grub_uint16_t  undi_code_seg;  /* UNDI Code segment address */
 +  grub_uint16_t  undi_code_size; /* UNDI Code segment size (bytes) */
 +  grub_uint32_t pxe_ptr; /* SEG:OFF to !PXE struct */
 +} __attribute__ ((packed));
>>>
>>> Can you GRUB-ify the comments here and below a bit?
>>
>> What do you mean by "GRUB-ify the comments" ?
>
> ".  */
>
> Although it is not too important for structs, I think :-)

Fixed and committed.

-- 
Bean


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

Re: [PATCH][RFC] gdb support in GRUB build system

[Colin D Bennett]

On Tue, 05 Aug 2008 17:05:57 +0200
Felix Zielcke <[EMAIL PROTECTED]> wrote:

> Am Dienstag, den 05.08.2008, 07:42 -0700 schrieb Colin D Bennett:
> > I following the GRUB 2 gdb debugging instructions this week and I
> > was successful in using QEMU and gdb to debug GRUB.  I wondered if
> > we could make this a bit easier -- primarily by making GRUB's build
> > system generate the '.elf' files with full symbol info without
> > requiring a patch.  Attached is a patch against GRUB 2 trunk (this
> > is the one from the GRUB 2 debugging guide, just fixed to apply on
> > the latest GRUB sources).
> > 
> Well I already brought this wiki page up on here, but nobody really
> cared about updating this.
> I even did the same as you updateing that patch to current SVN ;)
> 
> See here:
> http://lists.gnu.org/archive/html/grub-devel/2008-07/msg00585.html
> http://lists.gnu.org/archive/html/grub-devel/2008-08/msg00037.html
> 
> > Let me know what you all think. Thanks!
> 
> Thanks that at least you try to that GDB + Qemu stuff easier.
> 
> Even with the patches from Lubomir from May I wasn't able to get the
> symbols of the loaded modules.
> I have used that load_modules function which is in the new gdb-macro
> patch but it didn't work.
> And the gdb-stub didn't apply and I was unsure if it was safe to just
> adjust the values in the header file which was changed.
> 
> I think I'm not that good in this stuff to get it working, so it's
> very nice that you want to do it now :)

Interesting.  Well, I think I used the 'load_all_modules' function
instead of just 'load_modules'.  And IIRC, I first used the GRUB legacy
floppy image that was provided to chainload GRUB 2 over QEMU's built-in
tftp, but I think that directly booting GRUB 2 in QEMU for debugging
did not work... I'll have to try it again to verify that.

Cheers,
Colin


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

Re: [PATCH][RFC] gdb support in GRUB build system

[Felix Zielcke]

Am Dienstag, den 05.08.2008, 07:42 -0700 schrieb Colin D Bennett:
> I following the GRUB 2 gdb debugging instructions this week and I was
> successful in using QEMU and gdb to debug GRUB.  I wondered if we could
> make this a bit easier -- primarily by making GRUB's build system
> generate the '.elf' files with full symbol info without requiring a
> patch.  Attached is a patch against GRUB 2 trunk (this is the one
> from the GRUB 2 debugging guide, just fixed to apply on the latest
> GRUB sources).
> 
Well I already brought this wiki page up on here, but nobody really
cared about updating this.
I even did the same as you updateing that patch to current SVN ;)

See here:
http://lists.gnu.org/archive/html/grub-devel/2008-07/msg00585.html
http://lists.gnu.org/archive/html/grub-devel/2008-08/msg00037.html

> Let me know what you all think. Thanks!

Thanks that at least you try to that GDB + Qemu stuff easier.

Even with the patches from Lubomir from May I wasn't able to get the
symbols of the loaded modules.
I have used that load_modules function which is in the new gdb-macro
patch but it didn't work.
And the gdb-stub didn't apply and I was unsure if it was safe to just
adjust the values in the header file which was changed.

I think I'm not that good in this stuff to get it working, so it's very
nice that you want to do it now :)



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

[PATCH][RFC] gdb support in GRUB build system

[Colin D Bennett]

I following the GRUB 2 gdb debugging instructions this week and I was
successful in using QEMU and gdb to debug GRUB.  I wondered if we could
make this a bit easier -- primarily by making GRUB's build system
generate the '.elf' files with full symbol info without requiring a
patch.  Attached is a patch against GRUB 2 trunk (this is the one
from the GRUB 2 debugging guide, just fixed to apply on the latest
GRUB sources).

We could make it a 'configure' option like '--enable-debug' or
something, but it seems like minimal overhead to generate the .elf
files for gdb in addition to "real" GRUB files.

Let me know what you all think.  Thanks!

Regards,
Colin
=== modified file 'genmk.rb'
--- genmk.rb	2008-07-28 21:35:40 +
+++ genmk.rb	2008-08-05 14:36:06 +
@@ -101,10 +101,11 @@
 mod_obj = mod_src.suffix('o')
 defsym = 'def-' + @name.suffix('lst')
 undsym = 'und-' + @name.suffix('lst')
+exec = @name.suffix('elf')
 mod_name = File.basename(@name, '.mod')
 symbolic_name = mod_name.sub(/\.[^\.]*$/, '')
 
-"CLEANFILES += [EMAIL PROTECTED] #{mod_obj} #{mod_src} #{pre_obj} #{objs_str} #{undsym}
+"CLEANFILES += [EMAIL PROTECTED] #{mod_obj} #{mod_src} #{pre_obj} #{objs_str} #{undsym} #{exec}
 ifneq ($(#{prefix}_EXPORTS),no)
 CLEANFILES += #{defsym}
 DEFSYMFILES += #{defsym}
@@ -112,11 +113,14 @@
 MOSTLYCLEANFILES += #{deps_str}
 UNDSYMFILES += #{undsym}
 
[EMAIL PROTECTED]: #{pre_obj} #{mod_obj} $(TARGET_OBJ2ELF)
[EMAIL PROTECTED]: #{exec}
+	-rm -f $@
+	$(OBJCOPY) --strip-unneeded -K grub_mod_init -K grub_mod_fini -R .note -R .comment $^ $@
+
+#{exec}: #{pre_obj} #{mod_obj} $(TARGET_OBJ2ELF)
 	-rm -f $@
 	$(TARGET_CC) $(#{prefix}_LDFLAGS) $(TARGET_LDFLAGS) $(MODULE_LDFLAGS) -Wl,-r,-d -o $@ #{pre_obj} #{mod_obj}
 	if test ! -z $(TARGET_OBJ2ELF); then ./$(TARGET_OBJ2ELF) $@ || (rm -f $@; exit 1); fi
-	$(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 $@

=== modified file 'kern/main.c'
--- kern/main.c	2008-08-04 21:54:06 +
+++ kern/main.c	2008-08-05 14:36:06 +
@@ -122,6 +122,9 @@
   grub_print_error ();
 }
 
+void
+main (void) __attribute__ ((alias("grub_main")));
+
 /* The main routine.  */
 void
 grub_main (void)



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

Re: [PATCH] Fix color problem the grub-emu

[Bean]

On Tue, Aug 5, 2008 at 6:59 PM, Marco Gerards <[EMAIL PROTECTED]> wrote:
> Bean <[EMAIL PROTECTED]> writes:
>
>> On Tue, Aug 5, 2008 at 6:15 PM, Marco Gerards <[EMAIL PROTECTED]> wrote:
>>> Hi,
>>>
>>> Bean <[EMAIL PROTECTED]> writes:
>>>
 Currently, the color handling in grub-emu is broken, sometimes you see
 nothing on screen. This patch fix it, now variable menu_color_normal
 and menu_color_highlight works properly in grub-emu.

 2008-08-02  Bean  <[EMAIL PROTECTED]>

   * util/console.c (grub_console_cur_color): New variable.
   (grub_console_standard_color): Likewise.
   (grub_console_normal_color): Likewise.
   (grub_console_highlight_color): Likewise.
   (color_map): Likewise.
   (use_color): Likewise.
   (NUM_COLORS): New macro.
   (grub_ncurses_setcolorstate): Handle color properly.
   (grub_ncurses_setcolor): Don't change color here, just remember the
   settings, color will be set in grub_ncurses_setcolorstate.
   (grub_ncurses_getcolor): New function.
   (grub_ncurses_init): Initialize color pairs.
   (grub_term grub_ncurses_term): New member grub_ncurses_getcolor.
>>>
>>> This should be:
>>>
>>>(grub_ncurses_term): New member grub_ncurses_getcolor.
>>>
 --
 Bean

 diff --git a/util/console.c b/util/console.c
 index 8c9401c..53fc5d0 100644
 --- a/util/console.c
 +++ b/util/console.c
 @@ -41,6 +41,28 @@

  static int grub_console_attr = A_NORMAL;

 +grub_uint8_t grub_console_cur_color = 7;
>>>
>>> Any reason why this is not static?
>>
>> Hi,
>>
>> This files include , which define
>> grub_console_cur_color as global variable. Although grub-emu doesn't
>> use this variable itself, but if I define it as static, it will
>> generate a warning message.
>
> Ah, ok :-)

Committed.

-- 
Bean


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

Re: GRUB has a problem with a big grub.cfg

[Bean]

On Tue, Aug 5, 2008 at 6:26 PM, Marco Gerards <[EMAIL PROTECTED]> wrote:
> Hi,
>
> Bean <[EMAIL PROTECTED]> writes:
>
>> I have found the bug, it's caused by buffer overflown. In get_line
>> (normal/main.c), if the string length is multiple of 64, the ending \0
>> will overflow the buffer, this patch fix the problem:
>
> Looks fine to me.

It's already committed.

-- 
Bean


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

Re: [PATCH] Misc patches for grub2

[Bean]

On Tue, Aug 5, 2008 at 6:36 PM, Marco Gerards <[EMAIL PROTECTED]> wrote:
> Hi,
>
> Bean <[EMAIL PROTECTED]> writes:
>
>> This is a collection of miscellaneous patches, it includes:
>
> Please do not collect patches.  Independant changes can better go into
> independant patches.  Mails like this are easily overseen and so are
> important changes made by such patch.
>

ok, I'd remember this the next time, :-).

>> 1, move util/envblk.c to lib/envblk.c
>>
>> As envblk.c is used by module loadenv and tool grub-editenv, I think
>> it's better to move it to lib directory.
>
>
> For some reason I have some doubts about lib/.  But I do not have a
> better name in mind either.  What does belong in there?  Do you happen
> to have other names in mind as a suggestion? :-)

The files in this directory are used by both modules and utilities,
while files in util/ are only used by utilities.

I also sense that lib is a little strange, but I can't think of a
better name. Some alternative name could be: shared, common, helper.

>> 4. rename appleloader command to bootcamp
>>
>> The name appleloader may be a little confusing, bootcamp seems to be a
>> better choice.
>
> How about legacyloader or even legacy?  Isn't that what it does
> without actually using possibly trademarked names people are afraid of
> using?

There is a legacy boot protocol in efi, but apple doesn't use it.
Therefore, I think it's better to use a name to indicate that this is
apple related, to distinguish from the generic service.

-- 
Bean


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

Re: [PATCH] Add support for /dev/md/N style mdraid devices

[Felix Zielcke]

Am Dienstag, den 05.08.2008, 15:36 +0200 schrieb Felix Zielcke:
> Am Dienstag, den 05.08.2008, 15:03 +0200 schrieb Felix Zielcke:
> 
> > 
> > I moved it now to include/util/misc.h so in the future it can be used
> > for all utils.
> 
> I just recompiled it with an 'svn up' before and a full compile is
> broken with that way.
> I just include config.h now which defines this.
> 
I should really look more at the code before mailing it.
I forgot a & and I've now changed at few lines the spaces to a \t
What I don't get it why the .diff looks like there is only one space
more between some lines, but vi shows 2 on the real modified file.


2008-08-05  Felix Zielcke  <[EMAIL PROTECTED]>

* util/getroot.c: Include .
(grub_util_get_grub_dev): Rewritten.
Index: util/getroot.c
===
--- util/getroot.c	(Revision 1777)
+++ util/getroot.c	(Arbeitskopie)
@@ -17,6 +17,7 @@
  *  along with GRUB.  If not, see .
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -406,67 +407,40 @@
 
   switch (grub_util_get_dev_abstraction (os_dev))
 {
-case GRUB_DEV_ABSTRACTION_LVM:
-  grub_dev = xmalloc (strlen (os_dev) - 12 + 1);
+  case GRUB_DEV_ABSTRACTION_LVM:
 
-  strcpy (grub_dev, os_dev + 12);
+asprintf (&grub_dev, "%s", os_dev + sizeof ("/dev/mapper/") -1);
+break;
 
-  break;
+  case GRUB_DEV_ABSTRACTION_RAID:
 
-case GRUB_DEV_ABSTRACTION_RAID:
-  grub_dev = xmalloc (20);
+	if (os_dev[7] == '_' && os_dev[8] == 'd')
+	  {
+	/* This a partitionable RAID device of the form /dev/md_dNNpMM. */
 
-  if (os_dev[7] == '_' && os_dev[8] == 'd')
-	{
-	  const char *p;
+	char *p;
 
-	  /* This a partitionable RAID device of the form /dev/md_dNNpMM. */
-	  int i;
+	p = strchr (os_dev, 'p');
+	if (p)
+	  *p = ',';
 
-	  grub_dev[0] = 'm';
-	  grub_dev[1] = 'd';
-	  i = 2;
-	  
-	  p = os_dev + 9;
-	  while (*p >= '0' && *p <= '9')
-	{
-	  grub_dev[i] = *p;
-	  i++;
-	  p++;
-	}
+	asprintf (&grub_dev, "md%s", os_dev + sizeof ("/dev/md_d") -1);
+	  }
+	else if (os_dev[7] >= '0' && os_dev[7] <= '9')
+	  {
+	asprintf (&grub_dev, "md%s", os_dev + sizeof ("/dev/md") -1);
+	  }
+	else if (os_dev[7] == '/' && os_dev[8] >= '0' && os_dev[8] <= '9')
+	  {
+	asprintf (&grub_dev, "md%s", os_dev + sizeof ("/dev/md/") -1);
+	  }
+	else
+	  grub_util_error ("Unknown kind of RAID device `%s'", os_dev);
 
-	  if (*p == '\0')
-	grub_dev[i] = '\0';
-	  else if (*p == 'p')
-	{
-	  p++;
-	  grub_dev[i] = ',';
-	  i++;
+break;
 
-	  while (*p >= '0' && *p <= '9')
-		{
-		  grub_dev[i] = *p;
-		  i++;
-		  p++;
-		}
-
-	  grub_dev[i] = '\0';
-	}
-	  else
-	grub_util_error ("Unknown kind of RAID device `%s'", os_dev);
-	}
-  else if (os_dev[7] >= '0' && os_dev[7] <= '9')
-	{
-	  memcpy (grub_dev, os_dev + 5, 7);
-	  grub_dev[7] = '\0';
-	}
-  else
-	grub_util_error ("Unknown kind of RAID device `%s'", os_dev);
-
-  break;
-
-default:  /* GRUB_DEV_ABSTRACTION_NONE */
-  grub_dev = grub_util_biosdisk_get_grub_dev (os_dev);
+  default:  /* GRUB_DEV_ABSTRACTION_NONE */
+	grub_dev = grub_util_biosdisk_get_grub_dev (os_dev);
 }
 
   return grub_dev;
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Add support for /dev/md/N style mdraid devices

[Felix Zielcke]

Am Dienstag, den 05.08.2008, 15:03 +0200 schrieb Felix Zielcke:

> 
> I moved it now to include/util/misc.h so in the future it can be used
> for all utils.

I just recompiled it with an 'svn up' before and a full compile is
broken with that way.
I just include config.h now which defines this.



2008-08-05  Felix Zielcke  <[EMAIL PROTECTED]>

* util/getroot.c: Include .
(grub_util_get_grub_dev): Rewritten.
Index: util/getroot.c
===
--- util/getroot.c	(Revision 1777)
+++ util/getroot.c	(Arbeitskopie)
@@ -17,6 +17,7 @@
  *  along with GRUB.  If not, see .
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -406,67 +407,40 @@
 
   switch (grub_util_get_dev_abstraction (os_dev))
 {
-case GRUB_DEV_ABSTRACTION_LVM:
-  grub_dev = xmalloc (strlen (os_dev) - 12 + 1);
+  case GRUB_DEV_ABSTRACTION_LVM:
 
-  strcpy (grub_dev, os_dev + 12);
+asprintf (&grub_dev, "%s", os_dev + sizeof ("/dev/mapper/") -1);
+break;
 
-  break;
+  case GRUB_DEV_ABSTRACTION_RAID:
 
-case GRUB_DEV_ABSTRACTION_RAID:
-  grub_dev = xmalloc (20);
+if (os_dev[7] == '_' && os_dev[8] == 'd')
+	  {
+	/* This a partitionable RAID device of the form /dev/md_dNNpMM. */
 
-  if (os_dev[7] == '_' && os_dev[8] == 'd')
-	{
-	  const char *p;
+	char *p;
 
-	  /* This a partitionable RAID device of the form /dev/md_dNNpMM. */
-	  int i;
+	p = strchr (os_dev, 'p');
+	if (p)
+	  *p = ',';
 
-	  grub_dev[0] = 'm';
-	  grub_dev[1] = 'd';
-	  i = 2;
-	  
-	  p = os_dev + 9;
-	  while (*p >= '0' && *p <= '9')
-	{
-	  grub_dev[i] = *p;
-	  i++;
-	  p++;
-	}
+	asprintf (grub_dev, "md%s", os_dev + sizeof ("/dev/md_d") -1);
+	  }
+else if (os_dev[7] >= '0' && os_dev[7] <= '9')
+	  {
+	asprintf (&grub_dev, "md%s", os_dev + sizeof ("/dev/md") -1);
+	  }
+else if (os_dev[7] == '/' && os_dev[8] >= '0' && os_dev[8] <= '9')
+	  {
+	asprintf (&grub_dev, "md%s", os_dev + sizeof ("/dev/md/") -1);
+	  }
+else
+	  grub_util_error ("Unknown kind of RAID device `%s'", os_dev);
 
-	  if (*p == '\0')
-	grub_dev[i] = '\0';
-	  else if (*p == 'p')
-	{
-	  p++;
-	  grub_dev[i] = ',';
-	  i++;
+break;
 
-	  while (*p >= '0' && *p <= '9')
-		{
-		  grub_dev[i] = *p;
-		  i++;
-		  p++;
-		}
-
-	  grub_dev[i] = '\0';
-	}
-	  else
-	grub_util_error ("Unknown kind of RAID device `%s'", os_dev);
-	}
-  else if (os_dev[7] >= '0' && os_dev[7] <= '9')
-	{
-	  memcpy (grub_dev, os_dev + 5, 7);
-	  grub_dev[7] = '\0';
-	}
-  else
-	grub_util_error ("Unknown kind of RAID device `%s'", os_dev);
-
-  break;
-
-default:  /* GRUB_DEV_ABSTRACTION_NONE */
-  grub_dev = grub_util_biosdisk_get_grub_dev (os_dev);
+  default:  /* GRUB_DEV_ABSTRACTION_NONE */
+grub_dev = grub_util_biosdisk_get_grub_dev (os_dev);
 }
 
   return grub_dev;
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] skip over invalid BSD partitions

[Felix Zielcke]

Am Dienstag, den 05.08.2008, 15:10 +0200 schrieb Marco Gerards:
> Felix Zielcke <[EMAIL PROTECTED]> writes:
> 
> > 2008-08-05  Felix Zielcke  <[EMAIL PROTECTED]>
> >
> > * partmap/pc.c (pc_partition_map_iterate): Do not abort on an invalid 
> > BSD
> > magic or if there's no space left, use grub_dprintf to issue a warning.
> 
> Looks fine.
Hurray.

> > +   {
> > + grub_dprintf ("partition",
> > +   "invalid disk label magic 0x%x on partition 
> > %d\n"
> > +   label.magic, p.index);
> > + continue;
> > +   }
> 
> Isn't a comma missing after the second string?  I am surprised that
> this compiles.  Or am I missing something?
> 
You're right, shame on me I notice to missing spaces but not a missing
comma.
Attached.
Index: partmap/pc.c
===
--- partmap/pc.c	(Revision 1770)
+++ partmap/pc.c	(Arbeitskopie)
@@ -160,9 +160,10 @@
 		{
 		  /* Check if the BSD label is within the DOS partition.  */
 		  if (p.len <= GRUB_PC_PARTITION_BSD_LABEL_SECTOR)
-		return grub_error (GRUB_ERR_BAD_PART_TABLE,
-   "no space for disk label");
-
+		{
+		  grub_dprintf ("partition", "no space for disk label\n");
+		  continue;
+		}
 		  /* Read the BSD label.  */
 		  if (grub_disk_read (&raw,
   (p.start
@@ -175,10 +176,12 @@
 		  /* Check if it is valid.  */
 		  if (label.magic
 		  != grub_cpu_to_le32 (GRUB_PC_PARTITION_BSD_LABEL_MAGIC))
-		return grub_error (GRUB_ERR_BAD_PART_TABLE,
-   "invalid disk label magic 0x%x",
-   label.magic);
-
+		{
+		  grub_dprintf ("partition",
+"invalid disk label magic 0x%x on partition %d\n",
+label.magic, p.index);
+		  continue;
+		}
 		  for (pcdata.bsd_part = 0;
 		   pcdata.bsd_part < grub_cpu_to_le16 (label.num_partitions);
 		   pcdata.bsd_part++)
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] skip over invalid BSD partitions

[Felix Zielcke]

Am Dienstag, den 05.08.2008, 15:10 +0200 schrieb Marco Gerards:
> Felix Zielcke <[EMAIL PROTECTED]> writes:
> 
> > 2008-08-05  Felix Zielcke  <[EMAIL PROTECTED]>
> >
> > * partmap/pc.c (pc_partition_map_iterate): Do not abort on an invalid 
> > BSD
> > magic or if there's no space left, use grub_dprintf to issue a warning.
> 
> Looks fine.
Hurray.
> 
> > +   {
> > + grub_dprintf ("partition",
> > +   "invalid disk label magic 0x%x on partition 
> > %d\n"
> > +   label.magic, p.index);
> > + continue;
> > +   }
> 
> Isn't a comma missing after the second string?  I am surprised that
> this compiles.  Or am I missing something?
> 

Bah, I noticed the 2 missing spaces and then missed that one out :(
Attached.
Index: partmap/pc.c
===
--- partmap/pc.c	(Revision 1770)
+++ partmap/pc.c	(Arbeitskopie)
@@ -160,9 +160,10 @@
 		{
 		  /* Check if the BSD label is within the DOS partition.  */
 		  if (p.len <= GRUB_PC_PARTITION_BSD_LABEL_SECTOR)
-		return grub_error (GRUB_ERR_BAD_PART_TABLE,
-   "no space for disk label");
-
+		{
+		  grub_dprintf ("partition", "no space for disk label\n");
+		  continue;
+		}
 		  /* Read the BSD label.  */
 		  if (grub_disk_read (&raw,
   (p.start
@@ -175,10 +176,12 @@
 		  /* Check if it is valid.  */
 		  if (label.magic
 		  != grub_cpu_to_le32 (GRUB_PC_PARTITION_BSD_LABEL_MAGIC))
-		return grub_error (GRUB_ERR_BAD_PART_TABLE,
-   "invalid disk label magic 0x%x",
-   label.magic);
-
+		{
+		  grub_dprintf ("partition",
+"invalid disk label magic 0x%x on partition %d\n"
+label.magic, p.index);
+		  continue;
+		}
 		  for (pcdata.bsd_part = 0;
 		   pcdata.bsd_part < grub_cpu_to_le16 (label.num_partitions);
 		   pcdata.bsd_part++)
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Home-End keys in menu

[Marco Gerards]

Hi,

Carles Pina i Estany <[EMAIL PROTECTED]> writes:

> Some weeks ago I sent a patch that didn't have any discussion. I'm
> sending it again, maybe everybody was in holidays :-)
>
> Actually I updated the patch (added some spaces to comply with the
> coding style).
>
> * menu/normal.c: Add Home and End keys in grub-menu

The first line of the changelog entry is missing (name + e-mail
address).  Please mention the function you change like:

 * menu/normal.c (foo): Blah blah.

And end a sentence with a "."

>> Index: normal/menu.c
>> ===
>> --- normal/menu.c(revision 1718)
>> +++ normal/menu.c(working copy)
>> @@ -405,6 +405,22 @@
>>
>>switch (c)
>>  {
>> +case GRUB_TERM_HOME:
>> +   first=0;
>> +   offset=0;
>> +   print_entries (menu, first, offset);
>> +   break;

The indentation of the case statement doesn't seem right...

>> +case GRUB_TERM_END:
>> +  offset = menu->size - 1;
>> +  if (offset > GRUB_TERM_NUM_ENTRIES - 1)
>> +{
>> +  first = offset - (GRUB_TERM_NUM_ENTRIES - 1);
>> +  offset = GRUB_TERM_NUM_ENTRIES - 1;

This indentation also looks funny...

>> +}
>> +print_entries (menu, first, offset);
>> +  break;
>> +
>>  case 16:
>>  case '^':
>>if (offset > 0)
>
>> ___
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> http://lists.gnu.org/mailman/listinfo/grub-devel
>
> -- 
> Carles Pina i Estany  GPG id: 0x17756391
>   http://pinux.info
>
> ___
> 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: [PATCH] skip over invalid BSD partitions

[Marco Gerards]

Felix Zielcke <[EMAIL PROTECTED]> writes:

> 2008-08-05  Felix Zielcke  <[EMAIL PROTECTED]>
>
>   * partmap/pc.c (pc_partition_map_iterate): Do not abort on an invalid 
> BSD
>   magic or if there's no space left, use grub_dprintf to issue a warning.

Looks fine.

> + {
> +   grub_dprintf ("partition",
> + "invalid disk label magic 0x%x on partition 
> %d\n"
> + label.magic, p.index);
> +   continue;
> + }

Isn't a comma missing after the second string?  I am surprised that
this compiles.  Or am I missing something?

--
Marco



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

Re: [PATCH] update-grub for Cygwin

[Christian Franke]

Robert Millan wrote:

On Mon, Aug 04, 2008 at 10:40:02PM +0200, Christian Franke wrote:
  

Robert Millan wrote:


On Mon, Aug 04, 2008 at 09:46:03PM +0200, Christian Franke wrote:
 
  
Here a more generic version which allows to specifiy windows system dirs 
by /etc/default/grub:GRUB_WINDOWS_DIRS.


Defaults to current SYSTEMDRIVE on Cygwin, and nothing on other OS.

Christian

2008-08-04  Christian Franke  <[EMAIL PROTECTED]>

* conf/common.rmk: Add `10_windows' to `update-grub_SCRIPTS'.
* util/grub.d/10_windows.in: New file.
* util/update-grub.in: Add export of GRUB_WINDOWS_DIRS.
   

Why is this needed?  Can't we do something like 'grub-probe -t device 
c:/ntldr'

or so?

 
  

It is not needed for standard installations with ntldr on SYSTEMDRIVE (C:).
The ability to specify GRUB_WINDOWS_DIRS is added to support 
non-standard installations.

It also allows to use 10_windows on other OS if os-prober is not available.



Why not have the user write a custom entry then?  I think it clutters the
user interface to add options for everything.  If a corner case (boot a non
native disk, can't use os-prober) can be supported by creating a new config
file, why not do that?  It was the whole reason for designing update-grub to
be easily extensible.

  


Even with the new configuration parameter, user can still decide to 
leave the parameter unset and write a custom entry. Or the user can set 
the parameter and so "opt-in" to let the script do the work (or 
"opt-out" to disable the default work on Cygwin).


But in case the extra 'export' in 'update-grub' is a problem, here a 
simplified version.


Scans C: and SYSTEMDRIVE on Cygwin, does nothing elsewhere. Cannot be 
configured.


Christian

2008-08-05  Christian Franke  <[EMAIL PROTECTED]>

* conf/common.rmk: Add `10_windows' to `update-grub_SCRIPTS'.
* util/grub.d/10_windows.in: New file.


diff --git a/conf/common.rmk b/conf/common.rmk
index 3d3cd8a..3d674a6 100644
--- a/conf/common.rmk
+++ b/conf/common.rmk
@@ -120,7 +120,7 @@ CLEANFILES += update-grub_lib
 %: util/grub.d/%.in config.status
 	./config.status --file=$@:$<
 	chmod +x $@
-update-grub_SCRIPTS = 00_header 10_linux 10_hurd 30_os-prober 40_custom
+update-grub_SCRIPTS = 00_header 10_linux 10_hurd 10_windows 30_os-prober 40_custom
 CLEANFILES += $(update-grub_SCRIPTS)
 
 update-grub_DATA += util/grub.d/README
diff --git a/util/grub.d/10_windows.in b/util/grub.d/10_windows.in
new file mode 100644
index 000..e8f3c3e
--- /dev/null
+++ b/util/grub.d/10_windows.in
@@ -0,0 +1,83 @@
+#! /bin/sh -e
+
+# update-grub helper script.
+# Copyright (C) 2008  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 .
+
[EMAIL PROTECTED]@
[EMAIL PROTECTED]@
[EMAIL PROTECTED]@
+. ${libdir}/grub/update-grub_lib
+
+case "`uname 2>/dev/null`" in
+  CYGWIN*)  ;;
+  *) exit 0 ;;
+esac
+
+# Try C: even if current system is on other partition.
+case "$SYSTEMDRIVE" in
+  [Cc]:) dirlist="C:"  ;;
+  [D-Zd-z]:) dirlist="C: $SYSTEMDRIVE" ;;
+  *) exit 0 ;;
+esac
+
+get_os_name_from_boot_ini ()
+{
+  # Fail if no or more than one partition.
+  test "`sed -n 's,^\(\(multi\|scsi\)[^=]*\)=.*$,\1,p' "$1" 2>/dev/null | \
+sort | uniq | wc -l`" = 1 || return 1
+
+  # Search 'default=PARTITION'
+  local part=`sed -n 's,^default=,,p' "$1" | sed 's,,/,g;s,[ \t\r]*$,,;1q'`
+  test -n "$part" || return 1
+
+  # Search 'PARTITION="NAME" ...'
+  local name=`sed -n 's,,/,g;s,^'"$part"'="\([^"]*\)".*$,\1,p' "$1" | sed 1q`
+  test -n "$name" || return 1
+
+  echo "$name"
+}
+
+
+for dir in $dirlist ; do
+
+  # Check for Vista bootmgr.
+  if [ -f "$dir"/bootmgr -a -f "$dir"/boot/bcd ] ; then
+OS="Windows Vista bootmgr"
+
+  # Check for NTLDR.
+  elif [ -f "$dir"/ntldr -a -f "$dir"/ntdetect.com -a -f "$dir"/boot.ini ] ; then
+OS=`get_os_name_from_boot_ini "$dir"/boot.ini` || OS="Windows NT/2000/XP loader"
+
+  else
+continue
+  fi
+
+  # Get boot /dev/ice.
+  dev=`${grub_probe} -t device "$dir" 2>/dev/null` || continue
+
+  echo "Found $OS on $dir ($dev)" >&2
+  cat << EOF
+menuentry "$OS" {
+EOF
+
+  prepare_grub_to_access_device "$dev" | sed 's,^,\t,'
+
+  cat << EOF
+	chainloader +1
+}
+EOF
+done
+
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Home-End keys in menu

[Carles Pina i Estany]

Hello,

Some weeks ago I sent a patch that didn't have any discussion. I'm
sending it again, maybe everybody was in holidays :-)

Actually I updated the patch (added some spaces to comply with the
coding style).

* menu/normal.c: Add Home and End keys in grub-menu

Also, I added a comment that would be nice to change some magic numbers
to constants that already exists.

(I feel that it's a insignifcant patch compared with what I have seen
here!)

Thank you,

On Jul/21/2008, Carles Pina i Estany wrote:
> 
> 
> Hello,
> 
> Last weekend we talked about "menu loop" (wrapping):
> http://lists.gnu.org/archive/html/grub-devel/2008-07/msg00319.html
> 
> Conclusion: people here don't like it (we could discuss for ages, I
> think :-) )
> 
> Second proposal that maybe was hidden in so much text: to make it to
> work Home and End keys. Patch is attached. Do you need a more formal
> changelog for this? Adds Home and End key moving.
> Comments are welcomed.
> 
> Commend: I think that would be possible to change "case 14" by "case
> GRUB_TERM_DOWN"; and "case 16" by "case GRUB_TERM_UP" in normal/menu.c
> line 400 aprox. These constants are defined in include/grub/term.h. I
> don't send a patch because it's in the same "zone" than attached patch
> and it's easy-easy.
> 
> Thanks for your patience,
> 
> -- 
> Carles Pina i Estany  GPG id: 0x8CBDAE64
>   http://pinux.info   Manresa - Barcelona

> Index: normal/menu.c
> ===
> --- normal/menu.c (revision 1718)
> +++ normal/menu.c (working copy)
> @@ -405,6 +405,22 @@
> 
> switch (c)
>   {
> +case GRUB_TERM_HOME:
> +first=0;
> +offset=0;
> +print_entries (menu, first, offset);
> +break;
> +
> + case GRUB_TERM_END:
> +   offset = menu->size - 1;
> +   if (offset > GRUB_TERM_NUM_ENTRIES - 1)
> + {
> +  first = offset - (GRUB_TERM_NUM_ENTRIES - 1);
> +   offset = GRUB_TERM_NUM_ENTRIES - 1;
> + }
> + print_entries (menu, first, offset);
> +   break;
> +
>   case 16:
>   case '^':
> if (offset > 0)

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

-- 
Carles Pina i EstanyGPG id: 0x17756391
http://pinux.info
Index: normal/menu.c
===
--- normal/menu.c	(revision 1774)
+++ normal/menu.c	(working copy)
@@ -405,6 +405,22 @@
 	  
 	  switch (c)
 	{
+case GRUB_TERM_HOME:
+	   first = 0;
+	   offset = 0;
+	   print_entries (menu, first, offset);
+	   break;
+
+	case GRUB_TERM_END:
+	  offset = menu->size - 1;
+	  if (offset > GRUB_TERM_NUM_ENTRIES - 1)
+		{
+  first = offset - (GRUB_TERM_NUM_ENTRIES - 1);
+		  offset = GRUB_TERM_NUM_ENTRIES - 1;
+		}
+		print_entries (menu, first, offset);
+	  break;
+
 	case 16:
 	case '^':
 	  if (offset > 0)
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] High resolution time/TSC patch v3

[Marco Gerards]

Hi Colin,

Colin D Bennett <[EMAIL PROTECTED]> writes:

> Another updated TSC patch.  Now it detects TSC support for x86 CPUs at
> runtime and selects either the TSC or RTC time source.  This way 386
> and 486 CPUs without RDTSC instruction support are supported.
>
> Robert Millan was interested in getting this patch merged for the
> Coreboot port, so I decided to take another crack at it.
>
> Comments are welcome!

I have just committed your patch, with the ChangeLog changes I
proposed.  Please have a look to see what I changed.

--
Marco



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

Re: [PATCH] skip over invalid BSD partitions

[Felix Zielcke]

Am Dienstag, den 05.08.2008, 12:49 +0200 schrieb Felix Zielcke:
> Am Dienstag, den 05.08.2008, 12:16 +0200 schrieb Marco Gerards:
> > Robert Millan <[EMAIL PROTECTED]> writes:
> 
> > >
> > > How about using grub_dprintf instead?
> > 
> > Agreed.
> > 
> 
> Attached.

I forgot 2 little spaces ;)

> I hope I get soon used to your changelog :)
> 
> 
2008-08-05  Felix Zielcke  <[EMAIL PROTECTED]>

* partmap/pc.c (pc_partition_map_iterate): Do not abort on an invalid 
BSD
magic or if there's no space left, use grub_dprintf to issue a warning.

Index: partmap/pc.c
===
--- partmap/pc.c	(Revision 1770)
+++ partmap/pc.c	(Arbeitskopie)
@@ -160,9 +160,10 @@
 		{
 		  /* Check if the BSD label is within the DOS partition.  */
 		  if (p.len <= GRUB_PC_PARTITION_BSD_LABEL_SECTOR)
-		return grub_error (GRUB_ERR_BAD_PART_TABLE,
-   "no space for disk label");
-
+		{
+		  grub_dprintf ("partition", "no space for disk label\n");
+		  continue;
+		}
 		  /* Read the BSD label.  */
 		  if (grub_disk_read (&raw,
   (p.start
@@ -175,10 +176,12 @@
 		  /* Check if it is valid.  */
 		  if (label.magic
 		  != grub_cpu_to_le32 (GRUB_PC_PARTITION_BSD_LABEL_MAGIC))
-		return grub_error (GRUB_ERR_BAD_PART_TABLE,
-   "invalid disk label magic 0x%x",
-   label.magic);
-
+		{
+		  grub_dprintf ("partition",
+"invalid disk label magic 0x%x on partition %d\n"
+label.magic, p.index);
+		  continue;
+		}
 		  for (pcdata.bsd_part = 0;
 		   pcdata.bsd_part < grub_cpu_to_le16 (label.num_partitions);
 		   pcdata.bsd_part++)
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Drivemap module

[Marco Gerards]

Hi,

Javier Martín <[EMAIL PROTECTED]> writes:

>> Anyway, since "they" are more likely to maintain the code in 
>> the long run than you, in general, the question is whether 
>> the code is more likely to become buggy by their hacking on 
>> it, if it follows project coding style or someone else's 
>> (your) "safer" coding style.  Likely it's safer if using a 
>> consistent programming style.  Although I personally don't 
>> see that it's very helpful to have a style that makes things 
>> down to the order of "==" arguments be consistent within the 
>> project; haphazard only slows reading the tiniest bit, and I 
>> don't think it makes a different what order the arguments are...
> 
> Hmm... I was partially expecting a flamefest to start. Pity ^^
> Well, let's spill a little napalm: the GNU style bracing is extremely
> silly!! Why the hell are the "if" and "else" blocks indented differently
> in this example?
>   if (condition)
> return 0;
>   else
> {
>   return -1;
> }
> Nah, I'm not really bringing that issue, I was just joking, and in fact
> I'm reconsidering my objections to the operator== arguments order rule,
> even though I still consider my style safer and more sensible. If
> someone else wants to express their opinion on that issue, do it fast
> before I completely submit to Master Marco's will :D

Please don't be sarcastic, start flame wars or call names.  It will not
help anyone.

--
Marco



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

Re: [PATCH] Drivemap module

[Marco Gerards]

Hi,

Javier Martín <[EMAIL PROTECTED]> writes:

>> > There is, however, one point in which I keep my objection: comparisons
>> > between a variable and a constant should be of the form CONSTANT ==
>> > variable and not in the reverse order, since an erroneous but quite
>> > possible change of == for = results in a compile-time error instead of a
>> > _extremely_ difficult to trace runtime bug. Such kind of bugs are quite
>> > excruciating to find in userspace applications within an IDE, so I can't
>> > even consider the pain to debug them in a bootloader.
>> 
>> I understand your concern, nevertheless, can you please change it?
> You understand my concern, but seemingly do not understand that in order
> to conform to the Holy Coding Style you are asking me to write code that
> can become buggy (and with a very hard to trace bug) with a simple
> deltion! (point: did you notice that last word _without_ a spelling
> checker? Now try to do so in a multithousand-line program).

BTW, your patch still contains this, can you please change it before I
go over it again?

I know people who claim that this code will become buggy because we
write it in C.  Should we start accepting patches to rewrite GRUB in
Haskell or whatever? :-)

Really, as a maintainer I should set some standards and stick to it.
Of course not everyone will like me and sometimes I have to act like a
jerk.  But I rather be a jerk, than committing code that do not meet
my expectations.  But please understand, this contribution is highly
appreciated.  However, we want to have something maintainable for the
far future as well :-)

--
Marco



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

Re: [PATCH] Drivemap module

[Marco Gerards]

Hi Javier,

Javier Martín <[EMAIL PROTECTED]> writes:

> El lun, 04-08-2008 a las 22:51 +0200, Marco Gerards escribió:
>> Javier Martín <[EMAIL PROTECTED]> writes:
>> 
>> > After your latest replay, I "reevaluated" my stubbornness WRT some of
>> > your advices, and I've changed a few things:
>> >
>> > - Variables are now declared (and, if possible, initialized) before
>> > precondition checks, even simple ones. The install_int13_handler
>> > function has not been modified, however, since I find it a bit
>> > nonsensical to put bunch of declarations without an obvious meaning just
>> > after the "else" line:
>> >   grub_uint32_t *ivtslot;
>> >   grub_uint16_t *bpa_freekb;
>> >   grub_size_t total_size;
>> >   grub_uint16_t payload_sizekb;
>> >   grub_uint8_t *handler_base;
>> >   int13map_node_t *handler_map;
>> >   grub_uint32_t ivtentry;
>> 
>> Please understand me correctly.  Code just has to be written according
>> to our coding standards before it can and will be included.  We can
>> discuss things endlessly but we will simply stick to the GNU Coding
>> Standards as you might expect.
>> 
>> I hope you can appreciate that all code of GRUB has the same coding
>> style, if you like this style or not.
> Big sigh. Function modified to conform to your preciousss coding style.
> I might look a bit childish or arrogant saying this, but what is the
> point upholding a coding style that, no matter how consistent it is,
> hinders readability. With so much local variables, they are hard to keep
> track of no matter the coding style, but with the old ("mixed, heretic")
> style there was not need for a comment before each declaration: they
> were declared and used when needed instead of at the start of a block,
> because that function is inherently linear, not block-structured.

In your opinion it is not a good coding style.  I just avoid
discussions about it because such discussions just waste my time.
Even if you are able to convince me, GRUB is a GNU project and will
remain to use the GCS.


>> > - Only one declaration per line: even though C is a bit absurd in not
>> > recognizing T* as a first class type and instead thinking of * as a
>> > qualifier to the variable name; and even though my code does not incur
>> > into such ambiguities.
>> > - Comments moved as you required, reworded as needed
>> > - Extensive printf showing quasi-help in the "show" mode trimmed down to
>> > just the first sentence.
>> > - Internal helper functions now use the standard error handling, i.e.
>> > return grub_error (err, fmt, ...)
>> > - Comment about the strange "void" type instead of "void*" rephrased to
>> > be clearer
>> 
>> Thanks a lot!
>> 
>> > There is, however, one point in which I keep my objection: comparisons
>> > between a variable and a constant should be of the form CONSTANT ==
>> > variable and not in the reverse order, since an erroneous but quite
>> > possible change of == for = results in a compile-time error instead of a
>> > _extremely_ difficult to trace runtime bug. Such kind of bugs are quite
>> > excruciating to find in userspace applications within an IDE, so I can't
>> > even consider the pain to debug them in a bootloader.
>> 
>> I understand your concern, nevertheless, can you please change it?
> You understand my concern, but seemingly do not understand that in order
> to conform to the Holy Coding Style you are asking me to write code that
> can become buggy (and with a very hard to trace bug) with a simple
> deltion! (point: did you notice that last word _without_ a spelling
> checker? Now try to do so in a multithousand-line program).

Yes, I did notice it immediately.

The coding style is not holy in any way.  Everyone has its own coding
style.  You must understand I do not want to fully discuss it every
time someone sends in a patch, especially since it will not change
anyways.

> While tools like `svn diff' can help in these kind of problems, their
> utility is greatly diminished if the offending change is part of a
> multi-line change. Besides, sometimes checks like "if (a=b)", or more
> frequently "if (a=f())" are intentionally used in C, so the error might
> become even more difficult to spot and correct. I ask for a deep
> reflection on this issue: maybe I'm dead wrong and an arrogant brat in
> my attempt to tackle the Holy GNU Coding Standards, but I'd like to ask
> input from more people on this.

I will refuse patches with "if (a = f())", if that makes you sleep
better ;-)
 
>> > WRT accepting raw BIOS disk numbers, I agree with you in principle, but
>> > I'm keeping the functionality for now, since I don't quite like the
>> > "drivemap (hd0) (hd1)" syntax - which device is which?. I'd rather have
>> > something akin to "drivemap (hd0) (bios:hd1)", but I want to hear the
>> > opinions of people in this list.
>> 
>> I personally do not care a lot if BIOS disk numbers are used.
>> Although I do not see why it is useful.
>> 
>> As for the syntax, I would prefer s

Re: Bug#475718: Bug #475718 grub gets confused by hybrid apple/pc partmap

[Chris Knadle]

On Tuesday 05 August 2008, Marco Gerards wrote:
> Robert Millan <[EMAIL PROTECTED]> writes:
> > On Fri, Jul 25, 2008 at 07:36:25PM -0400, Chris Knadle wrote:
> >>If you ask me, I think this unfortunately looks like a complex
> >> detection problem -- which I think is eventually going to start with a
> >> *successful* detection of the Apple partition (because Apple + PC
> >> partitions can co-exist), followed by somehow *rejecting* that based on
> >> not finding any Apple HFS+ partitions (avoiding scanning partition 0),
> >> re-detecting partmaps (avoiding the failed Apple detection), and then
> >> going from there.  :-/  I've been thinking about that problem.  No
> >> matter how I look at this issue, it's messy.
> >
> > It can't be so complicated;  your layout is properly detected on Linux,
> > isn't it?
> >
> > Maybe we just need to give preference to MSDOS partitions.
> >
> > Btw, adding grub-devel to CC.  Let's try to have discussions in upstream
> > list..
>
> If I only know what you were talking about here... ;-)
>
> --
> Marco

   Who, me or Robert?  ;-)

   If you mean me -- I was confused.  Pavel made the suggestion of fixing the 
Grub2 issues with hybrid Apple/PC partition maps by searching for "the Apple 
magic" of 0x4552.  I've never dealt with Apple partition maps before, and so 
I didn't realize that Apple machines have a special "Apple" partition on the 
first partition of the drive -- so I mistakenly took Pavel's words to mean 
looking for an Apple *filesystem* (meaning searching on partitions > 0), so 
in my mind 0x4552 was for an HFS or HFS+ magic number.  I found some Apple 
documentatino that mentioned it, but if it mentioned that the 0x4552 magic 
was on partition 0, I missed it, so the logical train of thought led me to a 
bunch of erroneous conclusions.

   So yeah, if you don't understand what I wrote, it's because it's nonsense.

   Robert committed a fix for this bug and uploaded a new package, and the bug 
is closed.

   -- Chris

-- 

Chris Knadle
[EMAIL PROTECTED]


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

Re: [PATCH] Test for files with ls

[Marco Gerards]

Hi,

Lampersperger Andreas <[EMAIL PROTECTED]> writes:

> I have there a small patch to commands/ls.c, whichs makes the
> ls-command return an error for non existing fils (as the bash ls
> command).
>
> With this patch, one can have in the grub.cfg files statements like:
>
> if ls /boot/grub/grub.cfg.gfx ; then
> source /boot/grub/grub.cfg.gfx
> fi
>
> I would be glad if you accept the patch or give me comments to improve
> the patch.

It would be better to add a "test" command.  I have one sitting on my
harddisk, waiting for me to finish it... :-/

--
Marco



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

Re: [PATCH] Fix color problem the grub-emu

[Marco Gerards]

Bean <[EMAIL PROTECTED]> writes:

> On Tue, Aug 5, 2008 at 6:15 PM, Marco Gerards <[EMAIL PROTECTED]> wrote:
>> Hi,
>>
>> Bean <[EMAIL PROTECTED]> writes:
>>
>>> Currently, the color handling in grub-emu is broken, sometimes you see
>>> nothing on screen. This patch fix it, now variable menu_color_normal
>>> and menu_color_highlight works properly in grub-emu.
>>>
>>> 2008-08-02  Bean  <[EMAIL PROTECTED]>
>>>
>>>   * util/console.c (grub_console_cur_color): New variable.
>>>   (grub_console_standard_color): Likewise.
>>>   (grub_console_normal_color): Likewise.
>>>   (grub_console_highlight_color): Likewise.
>>>   (color_map): Likewise.
>>>   (use_color): Likewise.
>>>   (NUM_COLORS): New macro.
>>>   (grub_ncurses_setcolorstate): Handle color properly.
>>>   (grub_ncurses_setcolor): Don't change color here, just remember the
>>>   settings, color will be set in grub_ncurses_setcolorstate.
>>>   (grub_ncurses_getcolor): New function.
>>>   (grub_ncurses_init): Initialize color pairs.
>>>   (grub_term grub_ncurses_term): New member grub_ncurses_getcolor.
>>
>> This should be:
>>
>>(grub_ncurses_term): New member grub_ncurses_getcolor.
>>
>>> --
>>> Bean
>>>
>>> diff --git a/util/console.c b/util/console.c
>>> index 8c9401c..53fc5d0 100644
>>> --- a/util/console.c
>>> +++ b/util/console.c
>>> @@ -41,6 +41,28 @@
>>>
>>>  static int grub_console_attr = A_NORMAL;
>>>
>>> +grub_uint8_t grub_console_cur_color = 7;
>>
>> Any reason why this is not static?
>
> Hi,
>
> This files include , which define
> grub_console_cur_color as global variable. Although grub-emu doesn't
> use this variable itself, but if I define it as static, it will
> generate a warning message.

Ah, ok :-)

--
Marco



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

Re: [PATCH] skip over invalid BSD partitions

[Felix Zielcke]

Am Dienstag, den 05.08.2008, 12:16 +0200 schrieb Marco Gerards:
> Robert Millan <[EMAIL PROTECTED]> writes:

> >
> > How about using grub_dprintf instead?
> 
> Agreed.
> 

Attached.
I hope I get soon used to your changelog :)


2008-08-05  Felix Zielcke  <[EMAIL PROTECTED]>

* partmap/pc.c (pc_partition_map_iterate): Do not abort on an invalid 
BSD
magic or if there's no space left, use grub_dprintf to issue a warning.

Index: partmap/pc.c
===
--- partmap/pc.c	(Revision 1770)
+++ partmap/pc.c	(Arbeitskopie)
@@ -160,9 +160,10 @@
 		{
 		  /* Check if the BSD label is within the DOS partition.  */
 		  if (p.len <= GRUB_PC_PARTITION_BSD_LABEL_SECTOR)
-		return grub_error (GRUB_ERR_BAD_PART_TABLE,
-   "no space for disk label");
-
+		{
+		  grub_dprintf ("partition","no space for disk label\n");
+		  continue;
+		}
 		  /* Read the BSD label.  */
 		  if (grub_disk_read (&raw,
   (p.start
@@ -175,10 +176,12 @@
 		  /* Check if it is valid.  */
 		  if (label.magic
 		  != grub_cpu_to_le32 (GRUB_PC_PARTITION_BSD_LABEL_MAGIC))
-		return grub_error (GRUB_ERR_BAD_PART_TABLE,
-   "invalid disk label magic 0x%x",
-   label.magic);
-
+		{
+		  grub_dprintf ("partition",
+"invalid disk label magic 0x%x on partition %d\n"
+label.magic,p.index);
+		  continue;
+		}
 		  for (pcdata.bsd_part = 0;
 		   pcdata.bsd_part < grub_cpu_to_le16 (label.num_partitions);
 		   pcdata.bsd_part++)
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: Bug#475718: Bug #475718 grub gets confused by hybrid apple/pc partmap

[Marco Gerards]

Robert Millan <[EMAIL PROTECTED]> writes:

> On Fri, Jul 25, 2008 at 07:36:25PM -0400, Chris Knadle wrote:
>> 
>>If you ask me, I think this unfortunately looks like a complex detection 
>> problem -- which I think is eventually going to start with a *successful* 
>> detection of the Apple partition (because Apple + PC partitions can 
>> co-exist), followed by somehow *rejecting* that based on not finding any 
>> Apple HFS+ partitions (avoiding scanning partition 0), re-detecting partmaps 
>> (avoiding the failed Apple detection), and then going from there.  :-/  I've 
>> been thinking about that problem.  No matter how I look at this issue, it's 
>> messy.
>
> It can't be so complicated;  your layout is properly detected on Linux, isn't
> it?
>
> Maybe we just need to give preference to MSDOS partitions.
>
> Btw, adding grub-devel to CC.  Let's try to have discussions in upstream 
> list..

If I only know what you were talking about here... ;-)

--
Marco



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

Re: Remove conf/*.mk from svn

[Marco Gerards]

Hi,

Colin D Bennett <[EMAIL PROTECTED]> writes:

[...]

> I think we should remove conf/*.mk from the Subversion repository.  If
> people are going to be developing on GRUB and checking out svn
> branches, then I think it's fine to require them to have Ruby.  For
> released tarballs that we expect non-developers to use, we just need to
> generate the *.mk files and include them in the tarball.

I do not have problems with this.  Besides this, it will stop people
from sending in patches with .mk changes in it :-)

--
Marco



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

Re: Remove conf/*.mk from svn

[Marco Gerards]

Hi,

Christian Franke <[EMAIL PROTECTED]> writes:

> For a release tarball, It IMO also makes sense to include
> util/parser.tab.c. It is platform-independent and rarely changed. This
> would remove bison from the list of prerequisites.

Fine for me.

--
Marco



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

Re: two bugs in configfile parser

[Bean]

On Tue, Aug 5, 2008 at 6:17 PM, Marco Gerards <[EMAIL PROTECTED]> wrote:
> Hi,
>
> Bean <[EMAIL PROTECTED]> writes:
>
>> Ok, I've found the bug. In editor_getline (normal/menu_entry.c), it
>> should return a string allocated with grub_strdup, instead of the
>> original one, as the result will be release in the lexer once it's
>> done.
>
> The patch looks fine.  You can commit it, if you didn't do this
> already.

Oh, I have committed it.

-- 
Bean


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

Re: [PATCH] Misc patches for grub2

[Marco Gerards]

Hi,

Bean <[EMAIL PROTECTED]> writes:

> This is a collection of miscellaneous patches, it includes:

Please do not collect patches.  Independant changes can better go into
independant patches.  Mails like this are easily overseen and so are
important changes made by such patch.

> 1, move util/envblk.c to lib/envblk.c
>
> As envblk.c is used by module loadenv and tool grub-editenv, I think
> it's better to move it to lib directory.


For some reason I have some doubts about lib/.  But I do not have a
better name in mind either.  What does belong in there?  Do you happen
to have other names in mind as a suggestion? :-)

> 2. seperate hexdump function, and move it to lib/hexdump.c
>
> hexdump module consists of two parts, one is hexdump function, the
> other is user land command. I move the hexdump function to lib, as
> it's also used in other place, for example grub-fstest.

Same here :-)

> 3. add new command crc
>
> Just like hexdump, this module is split into two parts, lib/crc.c for
> the crc function, commands/crc.c for the user land command that
> calculate the crc checksum of selected file.

If it is for users, it should go into util/

When and how is it used?

> 4. rename appleloader command to bootcamp
>
> The name appleloader may be a little confusing, bootcamp seems to be a
> better choice.

How about legacyloader or even legacy?  Isn't that what it does
without actually using possibly trademarked names people are afraid of
using?

--
Marco




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

Re: [PATCH] Fix color problem the grub-emu

[Bean]

On Tue, Aug 5, 2008 at 6:15 PM, Marco Gerards <[EMAIL PROTECTED]> wrote:
> Hi,
>
> Bean <[EMAIL PROTECTED]> writes:
>
>> Currently, the color handling in grub-emu is broken, sometimes you see
>> nothing on screen. This patch fix it, now variable menu_color_normal
>> and menu_color_highlight works properly in grub-emu.
>>
>> 2008-08-02  Bean  <[EMAIL PROTECTED]>
>>
>>   * util/console.c (grub_console_cur_color): New variable.
>>   (grub_console_standard_color): Likewise.
>>   (grub_console_normal_color): Likewise.
>>   (grub_console_highlight_color): Likewise.
>>   (color_map): Likewise.
>>   (use_color): Likewise.
>>   (NUM_COLORS): New macro.
>>   (grub_ncurses_setcolorstate): Handle color properly.
>>   (grub_ncurses_setcolor): Don't change color here, just remember the
>>   settings, color will be set in grub_ncurses_setcolorstate.
>>   (grub_ncurses_getcolor): New function.
>>   (grub_ncurses_init): Initialize color pairs.
>>   (grub_term grub_ncurses_term): New member grub_ncurses_getcolor.
>
> This should be:
>
>(grub_ncurses_term): New member grub_ncurses_getcolor.
>
>> --
>> Bean
>>
>> diff --git a/util/console.c b/util/console.c
>> index 8c9401c..53fc5d0 100644
>> --- a/util/console.c
>> +++ b/util/console.c
>> @@ -41,6 +41,28 @@
>>
>>  static int grub_console_attr = A_NORMAL;
>>
>> +grub_uint8_t grub_console_cur_color = 7;
>
> Any reason why this is not static?

Hi,

This files include , which define
grub_console_cur_color as global variable. Although grub-emu doesn't
use this variable itself, but if I define it as static, it will
generate a warning message.

-- 
Bean


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

Re: GRUB has a problem with a big grub.cfg

[Marco Gerards]

Hi,

Bean <[EMAIL PROTECTED]> writes:

> I have found the bug, it's caused by buffer overflown. In get_line
> (normal/main.c), if the string length is multiple of 64, the ending \0
> will overflow the buffer, this patch fix the problem:

Looks fine to me.

--
Marco



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

Re: [PATCH] Add support for /dev/md/N style mdraid devices

[Marco Gerards]

Felix Zielcke <[EMAIL PROTECTED]> writes:

> Am Freitag, den 01.08.2008, 17:09 +0200 schrieb Felix Zielcke:
>
>> Maybe there's a way to make it even better so please comment all of you
>> grub-devels :)
>
> Thanks to Robert who suggested asprintf
>
> I hope the #define _GNU_SOURCE which is needed for it isn't a problem,
> you use anyway gcc specific extentions and GRUB is a GNU project :)

AFAIK it isn't, so better leave it away.

> I hadn't yet an idea how to solve that md_dNNpNN case with it, so I just
> moved the xmalloc() call down to that.

What's the problem?

> Please comment which way you mostly like.

I would prefer a changelog entry while commenting.

> Index: util/getroot.c
> ===
> --- util/getroot.c(Revision 1757)
> +++ util/getroot.c(Arbeitskopie)
> @@ -17,6 +17,7 @@
>   *  along with GRUB.  If not, see .
>   */
>  
> +#define _GNU_SOURCE
>  #include 
>  #include 
>  #include 
> @@ -407,17 +408,16 @@
>switch (grub_util_get_dev_abstraction (os_dev))
>  {
>  case GRUB_DEV_ABSTRACTION_LVM:
> -  grub_dev = xmalloc (strlen (os_dev) - 12 + 1);
>  
> -  strcpy (grub_dev, os_dev + 12);
> -
> +  asprintf(&grub_dev,"%s",os_dev + strlen("/dev/mapper/") -1);

Please add a space between asprintf and (&grub_dev 

Same for the lines below.

>break;
>  
>  case GRUB_DEV_ABSTRACTION_RAID:
> -  grub_dev = xmalloc (20);
>  
>if (os_dev[7] == '_' && os_dev[8] == 'd')
>   {
> +   grub_dev = xmalloc (20);
> +
> const char *p;
>  
> /* This a partitionable RAID device of the form /dev/md_dNNpMM. */
> @@ -457,9 +457,12 @@
>   }
>else if (os_dev[7] >= '0' && os_dev[7] <= '9')
>   {
> -   memcpy (grub_dev, os_dev + 5, 7);
> -   grub_dev[7] = '\0';
> +   asprintf(&grub_dev,"md%s",os_dev + sizeof("/dev/md") -1);
>   }
> +  else if (os_dev[7] == '/' && os_dev[8] >= '0' && os_dev[8] <= '9')
> + {
> +   asprintf(&grub_dev,"md%s",os_dev + sizeof("/dev/md/") -1);
> + }
>else
>   grub_util_error ("Unknown kind of RAID device `%s'", os_dev);
>  
> ___
> 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: two bugs in configfile parser

[Marco Gerards]

Hi,

Bean <[EMAIL PROTECTED]> writes:

> Ok, I've found the bug. In editor_getline (normal/menu_entry.c), it
> should return a string allocated with grub_strdup, instead of the
> original one, as the result will be release in the lexer once it's
> done.

The patch looks fine.  You can commit it, if you didn't do this
already.

--
Marco



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

Re: [PATCH] skip over invalid BSD partitions

[Marco Gerards]

Robert Millan <[EMAIL PROTECTED]> writes:

> On Fri, Aug 01, 2008 at 12:40:17PM +0200, Marco Gerards wrote:
>> > -  return grub_error (GRUB_ERR_BAD_PART_TABLE,
>> > - "no space for disk label");
>> > -
>> > +  {
>> > +grub_error (GRUB_ERR_BAD_PART_TABLE,
>> > +"no space for disk label");
>> > +continue;
>> > +  }
>> 
>> If you continue as no error occured, why do you throw an error?
>
> Uhm nobody's going to handle this error.  The caller will simply see that
> some partitions are not processed.
>
> How about using grub_dprintf instead?

Agreed.

--
Marco



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

Re: [PATCH] Fix color problem the grub-emu

[Marco Gerards]

Hi,

Bean <[EMAIL PROTECTED]> writes:

> Currently, the color handling in grub-emu is broken, sometimes you see
> nothing on screen. This patch fix it, now variable menu_color_normal
> and menu_color_highlight works properly in grub-emu.
>
> 2008-08-02  Bean  <[EMAIL PROTECTED]>
>
>   * util/console.c (grub_console_cur_color): New variable.
>   (grub_console_standard_color): Likewise.
>   (grub_console_normal_color): Likewise.
>   (grub_console_highlight_color): Likewise.
>   (color_map): Likewise.
>   (use_color): Likewise.
>   (NUM_COLORS): New macro.
>   (grub_ncurses_setcolorstate): Handle color properly.
>   (grub_ncurses_setcolor): Don't change color here, just remember the
>   settings, color will be set in grub_ncurses_setcolorstate.
>   (grub_ncurses_getcolor): New function.
>   (grub_ncurses_init): Initialize color pairs.
>   (grub_term grub_ncurses_term): New member grub_ncurses_getcolor.

This should be:

(grub_ncurses_term): New member grub_ncurses_getcolor.

> -- 
> Bean
>
> diff --git a/util/console.c b/util/console.c
> index 8c9401c..53fc5d0 100644
> --- a/util/console.c
> +++ b/util/console.c
> @@ -41,6 +41,28 @@
>  
>  static int grub_console_attr = A_NORMAL;
>  
> +grub_uint8_t grub_console_cur_color = 7;

Any reason why this is not static?



Otherwise, like usual, your patch looks fine at first sight.  If it
works and no one else has objections just commit it after making these
two changes :-)

--
Marco



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

Re: [PATCH] grub-mkelfimage

[Marco Gerards]

Robert Millan <[EMAIL PROTECTED]> writes:

> The elf version of grub-mkimage is multiplatform, so it makes no sense to
> keep it as arch-specific in the build system IMHO.
>
> This patch renames it to grub-mkelfimage, and arranges the build system &
> headers so that its build parameters are shared between i386-ieee1275 and
> i386-coreboot (and in the future, among powerpc-* platforms).
>
> Aside from making life easier for distributors (less duplicated space), the
> big advantage in doing this is that one can now install grub-coreboot at
> the same time as grub-pc without causing a file conflict.

Good idea.  Can you send in a changelog entry?

--
Marco



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

Re: ata.c and sata disks

[Marco Gerards]

Hi Marco,

Marco "Vega" Trucillo <[EMAIL PROTECTED]> writes:

> On Tue, 06 May 2008 21:57:37 +0200, Marco "Vega" Trucillo
> <[EMAIL PROTECTED]> wrote:
>> On Tue, 6 May 2008 17:05:53 +0200, Robert Millan <[EMAIL PROTECTED]> wrote:
>> 
>>> Marco Gerards sent a patch to add PCI support to ata.c, but it had some
>> bugs that nobody has looked into yet. 
>
> I looked on Gerard's code and seems that if there is bug is it on original
> code of ata.c not in patch. However I've not tried yet Gerard's pach
> beacuse I can't now, I only looked his code and
> compared with my. My code is still very disorganised
> (http://62.123.21.220/ata.c). I think that
> tomorrow I'll try to modify Gerard's code that is written well.

Did you find the time to have a look? :-)

I just committed my PCI+ATA code, so people can actually to continue
to work on this.

Thanks,
Marco



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

Re: [PATCH] Fix color problem the grub-emu

[Bean]

On Tue, Aug 5, 2008 at 3:54 PM, Vesa Jääskeläinen <[EMAIL PROTECTED]> wrote:
> Bean wrote:
>>
>> Hi,
>>
>> Currently, the color handling in grub-emu is broken, sometimes you see
>> nothing on screen. This patch fix it, now variable menu_color_normal
>> and menu_color_highlight works properly in grub-emu.
>
>> +
>> +static grub_uint8_t color_map[NUM_COLORS] =
>> +{
>> +  COLOR_BLACK,
>> +  COLOR_BLUE,
>> +  COLOR_GREEN,
>> +  COLOR_CYAN,
>> +  COLOR_RED,
>> +  COLOR_MAGENTA,
>> +  COLOR_YELLOW,
>> +  COLOR_WHITE
>> +};
>> +
>
> Doesn't also ncurses have 16 different colors for foreground and 8 for
> background? This is at least quite common for terminals.
>
> On some terminals there is one bit controlling background highlight colors
> or text blinking.

Hi,

I check my system, it only has 8 foreground and 8 background, the
highlight color can be enabled with the A_BOLD flag.

-- 
Bean


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

Re: [PATCH] PXE support for grub2

[Marco Gerards]

Bean <[EMAIL PROTECTED]> writes:

>>> +GRUB_MOD_INIT(pxe)
>>> +{
>>> +  (void) mod;/* To stop warning. */
>>> +
>>> +  grub_pxe_detect ();
>>> +  if (grub_pxe_pxenv)
>>> +{
>>> +  grub_disk_dev_register (&grub_pxe_dev);
>>> +  grub_fs_register (&grub_pxefs_fs);
>>
>> filesystems belong in fs/
>
> Perhaps I should place it in fs/i386/pc ?

I think it would make things clearer, although I am not too sure :-)

>>> +struct grub_pxenv
>>> +{
>>> +  grub_uint8_t signature[6]; /* 'PXENV+' */
>>> +  grub_uint16_t version; /* MSB = major, LSB = minor */
>>> +  grub_uint8_t length;   /* structure length */
>>> +  grub_uint8_t checksum; /* checksum pad */
>>> +  grub_uint32_t rm_entry;/* SEG:OFF to PXE entry point */
>>> +  grub_uint32_t  pm_offset;  /* Protected mode entry */
>>> +  grub_uint16_t pm_selector; /* Protected mode selector */
>>> +  grub_uint16_t stack_seg;   /* Stack segment address */
>>> +  grub_uint16_t  stack_size; /* Stack segment size (bytes) */
>>> +  grub_uint16_t bc_code_seg; /* BC Code segment address */
>>> +  grub_uint16_t  bc_code_size;   /* BC Code segment size (bytes) */
>>> +  grub_uint16_t  bc_data_seg;/* BC Data segment address */
>>> +  grub_uint16_t  bc_data_size;   /* BC Data segment size (bytes) */
>>> +  grub_uint16_t  undi_data_seg;  /* UNDI Data segment address */
>>> +  grub_uint16_t  undi_data_size; /* UNDI Data segment size (bytes) */
>>> +  grub_uint16_t  undi_code_seg;  /* UNDI Code segment address */
>>> +  grub_uint16_t  undi_code_size; /* UNDI Code segment size (bytes) */
>>> +  grub_uint32_t pxe_ptr; /* SEG:OFF to !PXE struct */
>>> +} __attribute__ ((packed));
>>
>> Can you GRUB-ify the comments here and below a bit?
>
> What do you mean by "GRUB-ify the comments" ?

".  */

Although it is not too important for structs, I think :-)

--
Marco



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

Re: [PATCH] Fix color problem the grub-emu

[Vesa Jääskeläinen]

Bean wrote:

Hi,

Currently, the color handling in grub-emu is broken, sometimes you see
nothing on screen. This patch fix it, now variable menu_color_normal
and menu_color_highlight works properly in grub-emu.



+
+static grub_uint8_t color_map[NUM_COLORS] =
+{
+  COLOR_BLACK,
+  COLOR_BLUE,
+  COLOR_GREEN,
+  COLOR_CYAN,
+  COLOR_RED,
+  COLOR_MAGENTA,
+  COLOR_YELLOW,
+  COLOR_WHITE
+};
+


Doesn't also ncurses have 16 different colors for foreground and 8 for 
background? This is at least quite common for terminals.


On some terminals there is one bit controlling background highlight 
colors or text blinking.





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

Re: menu loop (patch)

[Vesa Jääskeläinen]

Robert Millan wrote:

On Sun, Jul 20, 2008 at 09:21:47AM +0300, [EMAIL PROTECTED] wrote:
I also like no wrapping mode. But lets assume we have nice graphical menu 
with "cylinder" of menu entries that roll when you press key up/down.


Oh my god.  Tell me you're not planning to implement OpenGL in GRUB! ;-)


Well... OpenGL ES could be cool :)... but no... Cylinder effect with 
texts always facing to user is really easy to simulate to have 3D look.



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

Re: grub to help refund of pre-installations

[Vesa Jääskeläinen]

Yoshinori K. Okuji wrote:

On Saturday 12 July 2008 16:59:20 Michael Gorven wrote:

On Saturday 12 July 2008 16:39:21 Robert Millan wrote:

On Tue, Jul 08, 2008 at 08:24:33AM +0200, Michael Gorven wrote:

On Tuesday 08 July 2008 07:32:40 Yoshinori K. Okuji wrote:

This news, basically, says that my company will provide a solution to
activating pre-installed software in a computer, after entering an
access code with cryptography, using GRUB 2.

I have been working on adding support for encrypted partitions to
GRUB2. It includes a generic crypto module with numerous ciphers and
hashes, which may be useful to you. My patch[1] is still waiting for
further review and for some legal issues to be addressed.

I think what Okuji said means merging it in GRUB 2 is not a prerequisite
for this plan:

I agree. I was just pointing out that my work may be useful and could save
him some effort. His module could use the ciphers and hashes from my patch
instead of reimplementing them again.


Yes. I will use your patch. I also would like to consider whether or how we 
should incorporate your patch into the official repository. Since ciphers are 
useful for many different things, I think the question is only about the API 
or the code structure.


Hi,

Could you also think about different password hashing schemes and other 
user authenticity cases while designing this.


Thanks,
Vesa Jääskeläinen


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

Re: RFC: conf/i386.rmk

[Vesa Jääskeläinen]

Pavel Roskin wrote:

On Sun, 2008-08-03 at 21:30 +0200, Robert Millan wrote:

On Sun, Aug 03, 2008 at 09:28:39PM +0200, Robert Millan wrote:

Hi,

I'm thinking that we have quite a bit of duplicate stuff in each of the 4
i386 ports (cpuid, pci, serial, etc) that could well live in a
firmware-agnostic conf/i386.rmk file.

This would simplify things and make them easier to maintain.  What do you
think?

Btw, I think grub-emu could go there too.  The only arch-specific thing in
it AFAICS is cpuid.mod.


I tried moving more stuff to common.rmk many times but gave up every
time.  One of the reasons is that the sparc64 support is very
out-of-date and doesn't use common.rmk at all.  I cannot even test it
(well, I haven't tries hard).


If sparc64 support is out of date and the maintainer is nowhere to be 
seen then I think you can put that support to graveyard until someone 
comes up to update it. Putting all common stuff to every platform to 
common.rmk is a good way and the only way (in my opinion) to go forward.



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