Re: gettext: commands/*
Hi,
On Dec/07/2009, Carles Pina i Estany wrote:
> And then implement grub_printf_:
> -
> int
> grub_printf_ (const char *fmt, ...)
> {
> va_list ap;
> int ret;
>
> va_start (ap, fmt);
> ret = grub_vprintf (_(fmt), ap);
actually I would better call grub_printf here instead of grub_vprintf
--
Carles Pina i Estany
http://pinux.info
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Broken common.rmk change
On Sun, Dec 06, 2009 at 05:22:52PM -0800, David Miller wrote: > From: Robert Millan > Date: Sun, 6 Dec 2009 14:10:26 +0100 > > > grub-mkconfig generates a grub.cfg that relies on UUIDs instead > > of hardcoding, etc. > > Well, for one thing I don't use initrd's for my Linux kernels, > therefore specifying root devices via UUIDs isn't going to work. Then don't use that feature of grub-mkconfig (i.e. set GRUB_DISABLE_LINUX_UUID=false in /etc/default/grub). That's independent of device.map handling, though. -- Colin Watson [cjwat...@ubuntu.com] ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: mkrelpath doesn't do what it should...
Am Sonntag, den 06.12.2009, 23:30 -0800 schrieb David Miller: > I was trying to figure out why the kernel image paths generated > automatically for me by grub-mkconfig were not correct. > > I have /boot on a seperate partition, but in the generated config > files it uses paths like /boot/vmlinux-2632 etc. > > The problem is grub-mkrelpath and it's usage in the scrips such as > "10_linux". > > "/boot" is given to "grub-mkrelpath", and this results in > the identical "/boot" in rel_dirname. > > So all the paths emitted by 10_linux end up being "/boot" based > instead of "/" based. > > grub-mkrelpath seems to do the right thing if I pass it a full path, > f.e. giving it "/boot/vmlinux" emits the correct "/vmlinux" The one mount point case is now fixed in r1917 See Subject: `handling mount points in grub-mkrelpath' from me for the patch. > Casually inspecting make_system_path_relative_to_its_root() shows what > appears to be another bug. It seems to immediately break from the > loop when a different device number than "/"'s is seen, but what if I > have: > > /one/two/three > > Where / is /dev/hda1, /one/two is /dev/hda2 and /one/two/three is yet > another mount from /dev/hda3. It seems like the premature loop exit > in this function will give us the wrong result here. > > The loop needs to remember the string prefix point at which every > device number change occurs, and once the whole path has been > traversed it should unwind back to that point in order to emit > the result. Multiple mount points are an interesting case. Haven't thought about them. But they should be now handled correctly too. -- Felix Zielcke Proud Debian Maintainer and GNU GRUB developer ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Broken common.rmk change
David Miller wrote: > From: Robert Millan > Date: Sun, 6 Dec 2009 14:10:26 +0100 > > >> grub-mkconfig generates a grub.cfg that relies on UUIDs instead >> of hardcoding, etc. >> > > Well, for one thing I don't use initrd's for my Linux kernels, > therefore specifying root devices via UUIDs isn't going to work. > > The problem actually is the following: When we're in OS we know the OS device name but not the firmware device name. So we're able to generate correct root= for kernel command line but can only guess which names you would need for grub's set root=. The solution we're going for is to have UUID of fs stored in grub.cfg and go through all partitions on boot and find the right one nd set it to root variable. As for passing root= kernel parameter the decision of using device name or UUID= is based on heuristics (sigh) but you can disable them altogether in /etc/default/grub: # Uncomment if you don't want GRUB to pass "root=UUID=xxx" parameter to Linux #GRUB_DISABLE_LINUX_UUID=true > ___ > Grub-devel mailing list > Grub-devel@gnu.org > http://lists.gnu.org/mailman/listinfo/grub-devel > > -- Regards Vladimir 'φ-coder/phcoder' Serbinenko signature.asc Description: OpenPGP digital signature ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH]: Fix CDROM skipping in ieee1275 ofdisk iterator.
Committed. David Miller wrote: > I tried to surmise other ways to do this more cleanly, such as > resolving the path and looking for some device property inside of the > resulting node, but there simply isn't anything we can check for. > > If you find a cleaner way we can commit it later. I was thinking of resolving path to hardware one and then querying hardware directly but it may conflict with OF. What do you think? How much work would be needed to make ata.mod work on sparc64? Which other drivers we would need to reasonably replace ofdisk (which would still be available as fallback)? In dumps in addition to ide I see protocols FP, ESP, SCSI, FAS, ISP, ULSA, SAS, SoC. ESP looks like SBUS-only so it can be fallbacked to OF considering number of users. I have no idea what FP,FAS, ISP, ULSA, SAS, SoC stand for and even if they are real protocols or just some weird pathnames. As for SCSI we definitely want it and already have scsi.mod but not the encapsulator of SCSI commands -- Regards Vladimir 'φ-coder/phcoder' Serbinenko signature.asc Description: OpenPGP digital signature ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Broken common.rmk change
From: Colin Watson Date: Mon, 7 Dec 2009 10:20:16 + > On Sun, Dec 06, 2009 at 05:22:52PM -0800, David Miller wrote: >> From: Robert Millan >> Date: Sun, 6 Dec 2009 14:10:26 +0100 >> >> > grub-mkconfig generates a grub.cfg that relies on UUIDs instead >> > of hardcoding, etc. >> >> Well, for one thing I don't use initrd's for my Linux kernels, >> therefore specifying root devices via UUIDs isn't going to work. > > Then don't use that feature of grub-mkconfig (i.e. set > GRUB_DISABLE_LINUX_UUID=false in /etc/default/grub). That's independent > of device.map handling, though. grub-mkconfig actually seems to do this automatically for me. ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Broken common.rmk change
From: Vladimir '$B&U(B-coder/phcoder' Serbinenko Date: Mon, 07 Dec 2009 11:48:07 +0100 > The problem actually is the following: > When we're in OS we know the OS device name but not the firmware device > name. So we're able to generate correct root= for kernel command line > but can only guess which names you would need for grub's set root=. The > solution we're going for is to have UUID of fs stored in grub.cfg and go > through all partitions on boot and find the right one nd set it to root > variable. As for passing root= kernel parameter the decision of using > device name or UUID= is based on heuristics (sigh) but you can disable > them altogether in /etc/default/grub: > > # Uncomment if you don't want GRUB to pass "root=UUID=xxx" parameter to > Linux > #GRUB_DISABLE_LINUX_UUID=true UUIDs are convenient but can be very time consuming. Some sparc64 systems come with 10 or 20 block OpenFirmware device aliases, and we're going probe all of them, one by one, to look for the UUID. The firmware device name is so trivially obtainable, especially on ieee1275. That's what the grub-ofpathname tool I wrote does on sparc64. And it's what the 'ofpathname' script used by powerpc on grub does as well. ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH]: Fix CDROM skipping in ieee1275 ofdisk iterator.
From: Vladimir '$B&U(B-coder/phcoder' Serbinenko Date: Mon, 07 Dec 2009 12:05:24 +0100 > Committed. > David Miller wrote: >> I tried to surmise other ways to do this more cleanly, such as >> resolving the path and looking for some device property inside of the >> resulting node, but there simply isn't anything we can check for. >> >> > If you find a cleaner way we can commit it later. I was thinking of > resolving path to hardware one and then querying hardware directly but > it may conflict with OF. What do you think? I don't think that is wise. We would then need two mappings, OF --> OS and OS --> OF for device naming. And all of that complexity for what? This silly cdrom issue where a simple strncmp() is solving the problem quite well so far? There are conventions for alias names created by default by the firmware, so we should be OK with the patch I have written here. > How much work would be needed to make ata.mod work on sparc64? Probably a lot. > Which other drivers we would need to reasonably replace ofdisk (which > would still be available as fallback)? I don't think it's worthwhile to break away from ofdisk, it works quite well. ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: mkrelpath doesn't do what it should...
From: Felix Zielcke Date: Mon, 07 Dec 2009 11:39:22 +0100 > Am Sonntag, den 06.12.2009, 23:30 -0800 schrieb David Miller: >> I was trying to figure out why the kernel image paths generated >> automatically for me by grub-mkconfig were not correct. >> >> I have /boot on a seperate partition, but in the generated config >> files it uses paths like /boot/vmlinux-2632 etc. >> >> The problem is grub-mkrelpath and it's usage in the scrips such as >> "10_linux". >> >> "/boot" is given to "grub-mkrelpath", and this results in >> the identical "/boot" in rel_dirname. >> >> So all the paths emitted by 10_linux end up being "/boot" based >> instead of "/" based. >> >> grub-mkrelpath seems to do the right thing if I pass it a full path, >> f.e. giving it "/boot/vmlinux" emits the correct "/vmlinux" > > The one mount point case is now fixed in r1917 > See Subject: `handling mount points in grub-mkrelpath' from me for the > patch. Thanks I'll have a look. Locally for testing, I changed the 10_linux script to pass the complete image path name to the device root relative pathname resolver. That worked just as well. In fact I don't see why these scripts don't do that, and relatively resolve the /boot directory whatever seperately. ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Broken common.rmk change
David Miller wrote: > From: Vladimir 'φ-coder/phcoder' Serbinenko > Date: Mon, 07 Dec 2009 11:48:07 +0100 > > >> The problem actually is the following: >> When we're in OS we know the OS device name but not the firmware device >> name. So we're able to generate correct root= for kernel command line >> but can only guess which names you would need for grub's set root=. The >> solution we're going for is to have UUID of fs stored in grub.cfg and go >> through all partitions on boot and find the right one nd set it to root >> variable. As for passing root= kernel parameter the decision of using >> device name or UUID= is based on heuristics (sigh) but you can disable >> them altogether in /etc/default/grub: >> >> # Uncomment if you don't want GRUB to pass "root=UUID=xxx" parameter to >> Linux >> #GRUB_DISABLE_LINUX_UUID=true >> > > UUIDs are convenient but can be very time consuming. > > Some sparc64 systems come with 10 or 20 block OpenFirmware device > aliases, and we're going probe all of them, one by one, to look for > the UUID. > > The firmware device name is so trivially obtainable, especially on > ieee1275. That's what the grub-ofpathname tool I wrote does on > sparc64. And it's what the 'ofpathname' script used by powerpc on > grub does as well. > > I think we can go for the approach Seth Goldberg proposed: UUID with hint. First we check if hinted device has correct UUID and if so we return it, if not we do the search -- Regards Vladimir 'φ-coder/phcoder' Serbinenko signature.asc Description: OpenPGP digital signature ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH]: Fix CDROM skipping in ieee1275 ofdisk iterator.
David Miller wrote: > From: Vladimir 'φ-coder/phcoder' Serbinenko > Date: Mon, 07 Dec 2009 12:05:24 +0100 > > >> Committed. >> David Miller wrote: >> >>> I tried to surmise other ways to do this more cleanly, such as >>> resolving the path and looking for some device property inside of the >>> resulting node, but there simply isn't anything we can check for. >>> >>> >>> >> If you find a cleaner way we can commit it later. I was thinking of >> resolving path to hardware one and then querying hardware directly but >> it may conflict with OF. What do you think? >> > > I don't think that is wise. > > We would then need two mappings, OF --> OS and OS --> OF for > device naming. I don't understand why we would need such a mapping. I meant to use full OF path which gives us where device is attached physically and so we can connect to it. > And all of that complexity for what? This > silly cdrom issue where a simple strncmp() is solving the > problem quite well so far? > > There are conventions for alias names created by default by > the firmware, so we should be OK with the patch I have written > here. > > The problem is that if user has defined additional aliases we fall into original problem. >> How much work would be needed to make ata.mod work on sparc64? >> > > Probably a lot. > > It already works on MIPS and AFAICT only thing it needs to work is PCI. Do you mean that accessing PCI on sparc is a lot of work? -- Regards Vladimir 'φ-coder/phcoder' Serbinenko signature.asc Description: OpenPGP digital signature ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Fixed some typos
On Fri, Dec 04, 2009 at 10:31:54PM +0100, Robert Millan wrote: > On Wed, Dec 02, 2009 at 10:14:29AM +, Colin Watson wrote: > > On Wed, Dec 02, 2009 at 10:01:44AM +0100, Richard Hartmann wrote: > > > On Wed, Dec 2, 2009 at 09:47, Colin Watson wrote: > > > > Thanks for your patch. However, it appears to be against GRUB Legacy, > > > > which no longer has an upstream maintainer. The same typos aren't > > > > present in GRUB 2. > > > > > > Correct, GRUB 2 is fine. > > > > > > Personally, I think it's worth committing to legacy because _if_, for > > > whatever reason, there is another release, it will help a tiny bit. If > > > not, > > > no harm is done and no one needs to care. > > > > I'm not even sure how to go about committing to GRUB Legacy. :-) Is > > sftp://bzr.sv.gnu.org/srv/bzr/grub/trunk/grub-legacy/ likely to work, or > > is it meant to be a read-only mirror of something? > > It's not clear there'll be another release of GRUB Legacy, but if there is, > I'd use that URI. OK. I've committed the patch at the head of this thread, then. Thanks, Richard. -- Colin Watson [cjwat...@ubuntu.com] ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: gettext: normal/*
On Sun, Dec 06, 2009 at 12:28:11AM +, Carles Pina i Estany wrote: > Is it fine to have a translatable string ended with a space? I think > that it's fine (same case than \n, even thought in this case msgfmt is > not warning). In my experience as a translator and as a i18n coordinator, I know that due to the fact msgfmt does not warn about the lack of the trailing space, it's very easy for translators to miss it and end up with a translated string that lacks it. IME, if it's not difficult to get rid of it, it better to do it for safety, because it's a common translator mistake. -- Jordi Mallach Pérez -- Debian developer http://www.debian.org/ jo...@sindominio.net jo...@debian.org http://www.sindominio.net/ GnuPG public key information available at http://oskuro.net/ signature.asc Description: Digital signature ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Broken common.rmk change
On Sun, Dec 06, 2009 at 04:21:50PM -0800, David Miller wrote: > From: Robert Millan > Date: Sun, 6 Dec 2009 14:10:26 +0100 > > > We're actually in the process of getting rid of device.map; > > Meanwhile, please fix the regression you added to the tree. :-) I think this should do it. Please can you test? > And I'll look into what you've mentioned to see if that kind of scheme > will work properly on Sparc. Thanks! -- Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all." === modified file 'conf/common.rmk' --- conf/common.rmk 2009-11-25 23:10:02 + +++ conf/common.rmk 2009-12-07 14:03:09 + @@ -3,7 +3,13 @@ sbin_UTILITIES += grub-mkdevicemap grub_mkdevicemap_SOURCES = gnulib/progname.c util/grub-mkdevicemap.c \ util/deviceiter.c \ - util/devicemap.c util/misc.c + util/misc.c + +ifeq ($(target_cpu)-$(platform), sparc64-ieee1275) +grub_mkdevicemap_SOURCES += util/ieee1275/ofpath.c util/ieee1275/devicemap.c +else +grub_mkdevicemap_SOURCES += util/devicemap.c +endif # For grub-mkelfimage. bin_UTILITIES += grub-mkelfimage ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Broken common.rmk change
On Sun, Dec 06, 2009 at 05:22:52PM -0800, David Miller wrote: > From: Robert Millan > Date: Sun, 6 Dec 2009 14:10:26 +0100 > > > grub-mkconfig generates a grub.cfg that relies on UUIDs instead > > of hardcoding, etc. > > Well, for one thing I don't use initrd's for my Linux kernels, > therefore specifying root devices via UUIDs isn't going to work. I was referring to use of UUIDs internally by GRUB (e.g. the search command instances grub-mkconfig generates). We also pass UUIDs to Linux, but this can be disabled in /etc/default/grub. -- 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
Build failures on Ubuntu due to gettext
Ubuntu's GCC enables -Wformat-security by default. This causes GCC to
(IMO rightly!) complain about constructs such as this:
grub_printf (_("foo"));
... because it's all too easy for a translator to (usually accidentally)
insert % sequences which would cause printf to behave incorrectly. This
should instead be:
grub_printf ("%s", _("foo"));
Patch follows. I can't help thinking that this would be easier with a
grub_puts, but perhaps that isn't worth it given the relatively small
number of occurrences here?
Also, should the line in notify_execution_failure instead be:
- grub_printf (_("Failed to boot default entries.\n"));
+ grub_printf ("%s\n", _("Failed to boot default entries."));
... to get rid of the unsightly \n in this translated string?
2009-12-07 Colin Watson
* normal/menu_entry.c (run): Don't pass the result of gettext as
the first argument to grub_printf, appeasing -Wformat-security.
(grub_menu_entry_run): Likewise.
* normal/menu_text.c (grub_wait_after_message): Likewise.
(print_message): Likewise.
(notify_execution_failure): Likewise.
=== modified file 'normal/menu_entry.c'
--- normal/menu_entry.c 2009-12-05 11:25:07 +
+++ normal/menu_entry.c 2009-12-07 14:02:20 +
@@ -1000,7 +1000,7 @@ run (struct screen *screen)
grub_cls ();
grub_printf (" ");
- grub_printf (_("Booting a command list"));
+ grub_printf ("%s", _("Booting a command list"));
grub_printf ("\n\n");
@@ -1182,6 +1182,6 @@ grub_menu_entry_run (grub_menu_entry_t e
grub_print_error ();
grub_errno = GRUB_ERR_NONE;
grub_putchar ('\n');
- grub_printf (_("Press any key to continue..."));
+ grub_printf ("%s", _("Press any key to continue..."));
(void) grub_getkey ();
}
=== modified file 'normal/menu_text.c'
--- normal/menu_text.c 2009-12-05 11:25:07 +
+++ normal/menu_text.c 2009-12-07 14:02:45 +
@@ -40,7 +40,7 @@ void
grub_wait_after_message (void)
{
grub_putchar ('\n');
- grub_printf (_("Press any key to continue..."));
+ grub_printf ("%s", _("Press any key to continue..."));
(void) grub_getkey ();
grub_putchar ('\n');
}
@@ -206,7 +206,7 @@ entry is highlighted.");
if (nested)
{
grub_printf ("\n");
- grub_printf (_("ESC to return previous menu."));
+ grub_printf ("%s", _("ESC to return previous menu."));
}
}
}
@@ -655,7 +655,7 @@ notify_execution_failure (void *userdata
grub_errno = GRUB_ERR_NONE;
}
grub_printf ("\n ");
- grub_printf (_("Failed to boot default entries.\n"));
+ grub_printf ("%s", _("Failed to boot default entries.\n"));
grub_wait_after_message ();
}
Thanks,
--
Colin Watson [cjwat...@ubuntu.com]
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
[RFC] Dynamic device.map
As Robert said recently, we're trying to get rid of our reliance on
device.map. Right now, it is still necessary to at least have entries in
device.map for any disks you wish to use with GRUB, although they don't
have to be particularly sensible; any bidirectional mapping will do (at
least as long as the 'search' command works).
I'd like to make it possible to run with no device.map at all, since
that makes it possible to plug in new disks without having to
reconfigure GRUB. Here's a draft patch to do this. I haven't supplied a
ChangeLog entry yet since it isn't suitable for commit yet, as noted in
TODO comments, but I'd welcome comments on the approach here.
This does slow down (e.g.) grub-probe a little: on my laptop with a
single hard disk, the median time for three runs of './grub-probe
--target=drive /' is 0.09 seconds, while the median time for three runs
of './grub-probe --target=drive -m /boot/grub/device.map.invalid /' is
0.18 seconds. I expect it would be worse on machines with many disks.
For the most part, I doubt this is a significant problem (certainly not
in e.g. grub-setup), but it will probably make a difference in scripts
that call grub-probe many times, such as grub-mkconfig. I suggest that
those should check whether the default device.map exists, and, if not,
send the output of grub-mkdevicemap to a temporary file and use that as
an ephemeral device.map cache.
=== modified file 'conf/any-emu.rmk'
--- conf/any-emu.rmk2009-11-26 00:08:42 +
+++ conf/any-emu.rmk2009-12-07 13:31:59 +
@@ -40,7 +40,7 @@ grub_emu_SOURCES = commands/minicmd.c co
fs/befs.c fs/befs_be.c fs/tar.c \
\
util/console.c util/hostfs.c util/grub-emu.c util/misc.c\
- util/hostdisk.c util/getroot.c \
+ util/hostdisk.c util/getroot.c util/deviceiter.c\
\
disk/raid.c disk/raid5_recover.c disk/raid6_recover.c \
disk/mdraid_linux.c disk/dmraid_nvidia.c disk/lvm.c \
=== modified file 'conf/common.rmk'
--- conf/common.rmk 2009-11-25 23:10:02 +
+++ conf/common.rmk 2009-12-07 13:32:14 +
@@ -17,6 +17,7 @@ sbin_UTILITIES += grub-probe
util/grub-probe.c_DEPENDENCIES = grub_probe_init.h
grub_probe_SOURCES = gnulib/progname.c util/grub-probe.c \
util/hostdisk.c util/misc.c util/getroot.c \
+ util/deviceiter.c \
kern/device.c kern/disk.c kern/err.c kern/misc.c\
kern/parser.c kern/partition.c kern/file.c \
\
=== modified file 'conf/i386-pc.rmk'
--- conf/i386-pc.rmk2009-11-25 23:10:02 +
+++ conf/i386-pc.rmk2009-12-07 13:32:32 +
@@ -93,6 +93,7 @@ util/i386/pc/grub-mkimage.c_DEPENDENCIES
util/i386/pc/grub-setup.c_DEPENDENCIES = grub_setup_init.h
grub_setup_SOURCES = gnulib/progname.c \
util/i386/pc/grub-setup.c util/hostdisk.c \
+ util/deviceiter.c \
util/misc.c util/getroot.c kern/device.c kern/disk.c\
kern/err.c kern/misc.c kern/parser.c kern/partition.c \
kern/file.c kern/fs.c kern/env.c fs/fshelp.c\
=== modified file 'conf/sparc64-ieee1275.rmk'
--- conf/sparc64-ieee1275.rmk 2009-12-03 11:18:56 +
+++ conf/sparc64-ieee1275.rmk 2009-12-07 13:32:43 +
@@ -68,6 +68,7 @@ grub_mkimage_SOURCES = util/sparc64/ieee
# For grub-setup.
util/sparc64/ieee1275/grub-setup.c_DEPENDENCIES = grub_setup_init.h
grub_setup_SOURCES = util/sparc64/ieee1275/grub-setup.c util/hostdisk.c
\
+ util/deviceiter.c \
util/misc.c util/getroot.c kern/device.c kern/disk.c\
kern/err.c kern/misc.c kern/parser.c kern/partition.c \
kern/file.c kern/fs.c kern/env.c fs/fshelp.c\
=== modified file 'util/hostdisk.c'
--- util/hostdisk.c 2009-11-23 20:30:56 +
+++ util/hostdisk.c 2009-12-07 14:14:53 +
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -551,6 +552,59 @@ static struct grub_disk_dev grub_util_bi
};
static void
+make_dynamic_device_map (void)
+{
+ auto int NESTED_FUNC_ATTR process_device (const char *name, int is_floppy);
+
+ int NESTED_FUNC_ATTR process_device (const char *name, int is_floppy)
+ {
+int num_fd = 0;
+int num_hd = 0;
+int drive;
+struct stat st;
+
+drive = find_free_slot ();
+if (drive < 0)
+ grub_util_error ("%s: Map table size exceeded", name);
+
+/* TODO duplication from read_device_map */
+#ifdef __MINGW32__
+(void) st;
+if (grub_util_get_disk_size (name) == -1LL)
+#else
+if (stat (name, &st) == -1)
+#endif
+ {
+ grub_util_info ("Cannot stat `%s', skipping", name);
+ return 0;
+ }
+
+/* TODO abstract via something like grub_util_emit_devicemap_entry? */
+if (is_flop
my plan for Multiboot 2
Hi, We have an obvious problem with the Multiboot 2 loader: It's in severe bitrot. Nobody complains because nobody uses it, which is understandable given that nobody programs for MB2, because it's not ready (both in spec and in implementation), and we don't improve it because nobody complains, etc. You get the point :-) I think the approach that was taken has proven wrong. It might have worked with more manpower, but our time resources are scarce and we have other priorities. In my opinion, as things stand now it is best if Multiboot 2 is developed by piggybacking on Multiboot 1 rather than as an isolated effort. This idea is twofold: both in spec and in implementation. Here's my plan for Multiboot: - Release Multiboot 1 as a standalone package, with no modifications. I will do this soon when I find some free time (and I think Vladimir had some cleanup done to the package that isn't yet merged). - Release a new revision of Multiboot 1, with only modifications that don't alter the spec. I.e. GRUB Legacy continues to be compliant, and we don't change the signature. These modifications would basically cope with the fact that Multiboot 1 is also usable on non-BIOS platforms, take advantage of modern 64-bit types to define equivalent structures, resolve some ambiguities, etc. - Release a new version of Multiboot 1, with only the modifications necessary for it to support non-i386 CPUs. In principle, it should be possible to do this without affecting the i386 definitions. Hence GRUB Legacy continues to be compliant. - Make loader/i386/multiboot.c CPU-independant, to the extent that this is possible. The idea is that the most amount of code that can be reasonably shared, should be. - Release a new version of Multiboot, based on Multiboot 1, this time contemplating changes that break compatibility. Proposed changes must be accompanied with a patch for our loader before they're committed to the spec. The Multiboot 2 draft in the wiki is a good source of ideas for improvements, although not necessarily the only one. - Repeat last step untill we're satisfied with the result and can dub it as "Multiboot 2". -- 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] Introduce xasprintf
On Fri, Sep 04, 2009 at 06:47:17PM +0200, Neal H. Walfield wrote:
> At Wed, 2 Sep 2009 02:49:39 +0100,
> Colin Watson wrote:
> > +#ifndef HAVE_VASPRINTF
> > +
> > +int
> > +vasprintf (char **buf, const char *fmt, va_list ap)
> > +{
> > + /* Should be large enough. */
> > + *buf = xmalloc (512);
> > +
> > + return vsprintf (*buf, fmt, ap);
> > +}
> > +
> > +#endif
>
> Perhaps check that the number of characters is not more than 512 (if
> so, panic).
I agree it's suboptimal, but I'm not sure this is possible without
shipping our own vsprintf implementation, which I would like to avoid
(gnulib does this properly, but it's much larger). Still, note that my
patch didn't really add that xmalloc code - all I did was move it from
the asprintf implementation - so this can be dealt with in a further
change if we choose.
I've gone ahead and committed this, since it's now well past 1.97 and I
think this is a clear improvement.
Thanks,
--
Colin Watson [cjwat...@ubuntu.com]
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: my plan for Multiboot 2
Robert Millan, le Mon 07 Dec 2009 16:08:31 +0100, a écrit : > - Release a new revision of Multiboot 1, with only modifications that don't > alter the spec. I.e. GRUB Legacy continues to be compliant, and we don't > change the signature. These modifications would basically cope with the > fact that Multiboot 1 is also usable on non-BIOS platforms, take advantage > of modern 64-bit types to define equivalent structures, resolve some > ambiguities, etc. I'm wondering about the multiboot_mod_list structure & 64bits: would mod_start/end, cmdline and pad become 64bits? Samuel ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [RFC] Dynamic device.map
On Mon, Dec 07, 2009 at 02:28:06PM +, Colin Watson wrote: > As Robert said recently, we're trying to get rid of our reliance on > device.map. Right now, it is still necessary to at least have entries in > device.map for any disks you wish to use with GRUB, although they don't > have to be particularly sensible; any bidirectional mapping will do (at > least as long as the 'search' command works). > > I'd like to make it possible to run with no device.map at all, since > that makes it possible to plug in new disks without having to > reconfigure GRUB. Here's a draft patch to do this. I haven't supplied a > ChangeLog entry yet since it isn't suitable for commit yet, as noted in > TODO comments, but I'd welcome comments on the approach here. Ah, I hadn't noticed r1870, sorry. Am I right in believing that my patch is obsolete, then? -- Colin Watson [cjwat...@ubuntu.com] ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
autogen.sh warnings
I did a fresh checkout of revision 1917 and autogen.sh produces several warnings, although the configure and make stages are OK. I am using relatively new versions of autoconf and automake: automake (GNU automake) 1.11 autoconf (GNU Autoconf) 2.64 When building, a lot of warnings is a red flag to me. I've studied the code and am attaching the fixes. autogen.sh now runs cleanly for me with a few informational messages. Below are explanations of what was going on and attached are patches to remove all the warnings. -- Bruce acinclude.m4:17: warning: underquoted definition of grub_PROG_TARGET_CC acinclude.m4:17: run info '(automake)Extending aclocal' acinclude.m4:17: or see http://sources.redhat.com/automake/automake.html#Extending-aclocal This can be fixed by adding [] around AC_DEFUN values in acinclude.m4, e.g.: AC_DEFUN(grub_PROG_TARGET_CC, needs to be changed to AC_DEFUN([grub_PROG_TARGET_CC], -- 16 places. - configure.ac:42: warning: AC_ARG_PROGRAM was called before AC_CANONICAL_TARGET ../../lib/autoconf/general.m4:1839: AC_CANONICAL_TARGET is expanded from... configure.ac:42: the top level configure.ac:42: warning: AC_ARG_PROGRAM was called before AC_CANONICAL_TARGET ../../lib/autoconf/general.m4:1839: AC_CANONICAL_TARGET is expanded from... configure.ac:42: the top level configure.ac:42: warning: AC_ARG_PROGRAM was called before AC_CANONICAL_TARGET ../../lib/autoconf/general.m4:1839: AC_CANONICAL_TARGET is expanded from... configure.ac:42: the top level configure.ac:42: warning: AC_ARG_PROGRAM was called before AC_CANONICAL_TARGET ../../lib/autoconf/general.m4:1839: AC_CANONICAL_TARGET is expanded from... configure.ac:42: the top level This can be fixed by moving line AM_INIT_AUTOMAKE() to right after the line AC_CANONICAL_TARGET. -- configure.ac:176: required file `./config.rpath' not found The can be fixed by `touch config.rpath` - automake: no `Makefile.am' found for any configure output I created a one line Makefile.am with the contents: SUBDIRS = . po This also fixes a warning generated by AM_GNU_GETTEXT which needed to be changed to AM_GNU_GETTEXT([external]) to avoid a complaint about a missing intl/ directory. Adding an empty ABOUT-NLS was also needed. - WARNING: C file isn't a module: hash-common.c WARNING: C file isn't a module: pubkey.c WARNING: C file isn't a module: ecc.c WARNING: C file isn't a module: elgamal.c WARNING: C file isn't a module: hmac-tests.c WARNING: C file isn't a module: md.c WARNING: C file isn't a module: rsa.c WARNING: C file isn't a module: ac.c WARNING: C file isn't a module: dsa.c WARNING: C file isn't a module: primegen.c WARNING: C file isn't a module: cipher.c These are generated by util/import_gcry.py by copying (and in some cases modifying) the files in lib/libgcrypt to lib/libgcrypt-grub. It also creates cipher.h, memory.h, and types.h. The warnings above appear to be spurious for autogen.sh I added code to util/import_gcry.py supress the warnings if a third parameter is passed. I then changed autogen.sh to use: python util/import_gcry.py lib/libgcrypt/ . NoWarn In a release, the lib/libgcrypt-grub/ directory should be released and the lib/libgcrypt/ should probably be suppressed to avoid dependence on ruby for non-developers. === added file 'ABOUT-NLS' === modified file 'ChangeLog' --- ChangeLog 2009-12-06 09:20:01 + +++ ChangeLog 2009-12-07 17:19:50 + @@ -1,3 +1,18 @@ +2009-12-07 Bruce Dubbs + + Remove spurious warnings when running autogen.sh. + + * util/import_gcry.py: Add optional third parameter to supress spurious + warnings. + * autogen.sh: Add third parameter to invocation of util/import_gcry.py. + * acinclude.m4: Add quotes to underquoted variables. + * configure.ac: Move AM_INIT_AUTOMAKE() to below AC_CANONICAL_TARGET + to supress warnings. Also changed to AM_GNU_GETTEXT([external]). + * Makefile.am: Add one line file to supress spurious warnings. + * config.rpath: Add empty file to prevent complaint by automake. + * ABOUT-NLS: Add empty file wanted by AM_GNU_GETTEXT. + + 2009-12-06 Felix Zielcke * util/misc.c (make_system_path_relative_to_its_root): Correctly cope with === added file 'Makefile.am' --- Makefile.am 1970-01-01 00:00:00 + +++ Makefile.am 2009-12-07 09:05:57 + @@ -0,0 +1,1 @@ +SUBDIRS = . po === modified file 'acinclude.m4' --- acinclude.m42009-11-16 19:31:29 + +++ acinclude.m42009-12-07 08:55:44 + @@ -14,7 +14,7 @@ dnl Check whether target compiler is working -AC_DEFUN(grub_PROG_TARGET_CC, +AC_DEFUN([grub_PROG_TARGET_CC], [AC_MSG_CHECKING([whether target compiler is working]) AC_CACHE_VAL(grub_cv_prog_target_cc, [AC_LINK_IFELSE([AC_LANG_PROGRAM([[ @@ -36,7 +36,7 @@ dnl compiling to assembler. dnl Written by Pavel Roskin. Based on grub_ASM_EXT_C written by dnl Erich Boleyn and modified by Yoshinori K. Okuji. -AC_D
Re: autogen.sh warnings
On Mon, Dec 07, 2009 at 11:38:06AM -0600, Bruce Dubbs wrote: > configure.ac:176: required file `./config.rpath' not found > > The can be fixed by `touch config.rpath` configure does actually run this, so I'd recommend copying the file from gettext or gnulib instead. > automake: no `Makefile.am' found for any configure output > > I created a one line Makefile.am with the contents: > > SUBDIRS = . po > > This also fixes a warning generated by AM_GNU_GETTEXT which needed to be > changed to AM_GNU_GETTEXT([external]) to avoid a complaint about a > missing intl/ directory. Adding an empty ABOUT-NLS was also needed. This is likely to be delicate since we're not actually using Automake as such, and we already have a separate Makefile.in which needs to not be overwritten. I think adding a Makefile.am would make it just too easy to clobber that by accident, even if it does suppress a warning message. -- Colin Watson [cjwat...@ubuntu.com] ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH]: Fix CDROM skipping in ieee1275 ofdisk iterator.
From: Vladimir '$B&U(B-coder/phcoder' Serbinenko Date: Mon, 07 Dec 2009 12:52:01 +0100 > It already works on MIPS and AFAICT only thing it needs to work is PCI. > Do you mean that accessing PCI on sparc is a lot of work? Even just calculating the register addresses is a lot of work. You can't just take the property values for the device as-is, you have to walk up to the root applying 'range' property translations to the register addresses. And there are special cases and quirks all over the place. The code to do this in the Linux kernel is several thousand lines of code. ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Broken common.rmk change
From: Robert Millan Date: Mon, 7 Dec 2009 15:05:16 +0100 > On Sun, Dec 06, 2009 at 05:22:52PM -0800, David Miller wrote: >> From: Robert Millan >> Date: Sun, 6 Dec 2009 14:10:26 +0100 >> >> > grub-mkconfig generates a grub.cfg that relies on UUIDs instead >> > of hardcoding, etc. >> >> Well, for one thing I don't use initrd's for my Linux kernels, >> therefore specifying root devices via UUIDs isn't going to work. > > I was referring to use of UUIDs internally by GRUB (e.g. the search command > instances grub-mkconfig generates). Ok, but as noted in another thread this can make grub very slow on ieee1275 systems as we are going to look for the UUID on every ieee1275 device alias present and there can be tons of those. ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: autogen.sh warnings
Bruce Dubbs wrote: > I did a fresh checkout of revision 1917 and autogen.sh produces > several warnings, although the configure and make stages are OK. I am > using relatively new versions of autoconf and automake: > > automake (GNU automake) 1.11 > autoconf (GNU Autoconf) 2.64 > > When building, a lot of warnings is a red flag to me. I've studied > the code and am attaching the fixes. autogen.sh now runs cleanly for > me with a few informational messages. > > Below are explanations of what was going on and attached are patches > to remove all the warnings. > Thanks. I'll let someone more familiar with automake to review automake parts. Just a comment to import_gcry > > These are generated by util/import_gcry.py by copying (and in some > cases modifying) the files in lib/libgcrypt to lib/libgcrypt-grub. It > also creates cipher.h, memory.h, and types.h. > > The warnings above appear to be spurious for autogen.sh crypto and experimental branches have newer import_gcry.py which has less warning by skipping copying files unnecessary for grub2. If you want to fix remaining warnings (on rsa, dsa, elgamal, primegen and ecc) implement assymetric cryptography support (it's planned but may need changes to core to be useful, there are lower hanging fruits for crypto). > > I added code to util/import_gcry.py supress the warnings if a third > parameter is passed. I then changed autogen.sh to use: > > python util/import_gcry.py lib/libgcrypt/ . NoWarn This NoWarn is just suppressing legitimate warnings > > In a release, the lib/libgcrypt-grub/ directory should be released and > the lib/libgcrypt/ should probably be suppressed to avoid dependence > on ruby for non-developers. import_gcry.py is python, not ruby. Generally ./autogen.sh is ran before releasing tarball -- Regards Vladimir 'φ-coder/phcoder' Serbinenko signature.asc Description: OpenPGP digital signature ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Build failures on Ubuntu due to gettext
Hello,
On Dec/07/2009, Colin Watson wrote:
> Ubuntu's GCC enables -Wformat-security by default. This causes GCC to
> (IMO rightly!) complain about constructs such as this:
>
> grub_printf (_("foo"));
I see...
(actually some weeks ago I thought about gettext security implications,
and I thought that if someone can change the .mo files there is some
bigger problem for the user... but I understand the point)
> ... because it's all too easy for a translator to (usually
> accidentally) insert % sequences which would cause printf to behave
(side and non-relevant note: if the translator wants to do some damage on
purpose he doesn't need to play with %s and %d, it's as easy as changing
strings from "Press C to Cancel and D to delete" to something like (in another
language) "Press D to Cancel and C to delete")
> incorrectly. This should instead be:
>
> grub_printf ("%s", _("foo"));
I like the general idea but I'm not 100% convinced of the
implementation:
a) It's a bit of false security because it's not fixing the case of file
normal/menu_text.c, line 191 (search the string "Use the %C and %C keys
to") or menu_text.c line 374 (search for "The highlighted entry")
(I agree that it's better than before... but not very solid)
b) How would you translate and handle:
grub_printf (_("Hello %s"), name);
The translator really needs "%s" because in other languages can be "%s
hello" (not the best example but maybe you get the point)
c) I'm thinking to implement grub_printf_ (str). I will send a patch
later to see if everybody likes. Then the call for simple strings would
be:
grub_printf_ ("%s", N_("Hello"));
Not much different from:
grub_printf_ (N_("Hello"));
But it's getting hard to print a string :-)
d) (important, even if it's the last one):
How it prevents mistakes from the current msgfmt checks?
For example, and using the option -c in msgfmt:
#: normal/misc.c:67
#, c-format
msgid "test %s t"
msgstr "test %d test2"
/usr/bin/msgfmt -c --statistics -o po/ca.mo po/ca.po
po/ca.po:1183: format specifications in 'msgid' and 'msgstr' for argument 1 are
not the same
/usr/bin/msgfmt: found 1 fatal error
26 translated messages, 213 untranslated messages.
Using any different number of %X from msgid and msgstr ishalting msgfmt (so, if
msgid contains 1 %s and 2 %d, msgstr has to contain the same)
We are talking from _(" "), I see that -Wformat-security is for "string came
from untrusted input and contains" when .mo are trusted. Are they?
How are other projectes implementing it? Specially the dynamic strings.
> Patch follows. I can't help thinking that this would be easier with a
> grub_puts, but perhaps that isn't worth it given the relatively small
> number of occurrences here?
We could replace grub_puts with grub_printf... or some other idea.
> Also, should the line in notify_execution_failure instead be:
>
> - grub_printf (_("Failed to boot default entries.\n"));
> + grub_printf ("%s\n", _("Failed to boot default entries."));
>
> ... to get rid of the unsightly \n in this translated string?
I really like that this fix the \n discussion that we had :-)
I like the idea and I understand that it's easy to make mistakes
translating strings. Actually I'm surprised that msgfmt is not giving
any warning if the
--
Carles Pina i Estany
http://pinux.info
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Broken common.rmk change
On Mon, Dec 07, 2009 at 11:50:33AM -0800, David Miller wrote: > From: Robert Millan > Date: Mon, 7 Dec 2009 15:05:16 +0100 > > > On Sun, Dec 06, 2009 at 05:22:52PM -0800, David Miller wrote: > >> From: Robert Millan > >> Date: Sun, 6 Dec 2009 14:10:26 +0100 > >> > >> > grub-mkconfig generates a grub.cfg that relies on UUIDs instead > >> > of hardcoding, etc. > >> > >> Well, for one thing I don't use initrd's for my Linux kernels, > >> therefore specifying root devices via UUIDs isn't going to work. > > > > I was referring to use of UUIDs internally by GRUB (e.g. the search command > > instances grub-mkconfig generates). > > Ok, but as noted in another thread this can make grub > very slow on ieee1275 systems as we are going to look > for the UUID on every ieee1275 device alias present and > there can be tons of those. I'm afraid I missed that thread. In any case, we basically do/can do three things to paliate that problem: - Cache the prepare_grub_to_access_device() calls at the shell/mkconfig layer (see util/grub.d/10_linux.in for an example). - Disk cache (kern/disk.c). - Adjust disk iteration order so that disks more likely to be the ones we're looking for show up first (e.g. on i386-pc we list hard disks before floppies or CD). Note that UUID search aborts as soon as a match is found; it doesn't iterate the whole list. More ideas welcome, of course :-) -- 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: autogen.sh warnings
Vladimir 'φ-coder/phcoder' Serbinenko wrote: Thanks. I'll let someone more familiar with automake to review automake parts. Just a comment to import_gcry These are generated by util/import_gcry.py by copying (and in some cases modifying) the files in lib/libgcrypt to lib/libgcrypt-grub. It also creates cipher.h, memory.h, and types.h. The warnings above appear to be spurious for autogen.sh crypto and experimental branches have newer import_gcry.py which has less warning by skipping copying files unnecessary for grub2. If you want to fix remaining warnings (on rsa, dsa, elgamal, primegen and ecc) implement assymetric cryptography support (it's planned but may need changes to core to be useful, there are lower hanging fruits for crypto). I'm not following you completely. I am trying to clean up the build environment, but not getting into the actual code. I think the crypto was just recently merged into trunk, but I'm not knowledgeable about the details yet to make any substantive changes. I added code to util/import_gcry.py supress the warnings if a third parameter is passed. I then changed autogen.sh to use: python util/import_gcry.py lib/libgcrypt/ . NoWarn This NoWarn is just suppressing legitimate warnings Is a warning that you can't create a subdirectory because it already exists a legitimate warning? The warning about files being copied not being modules seemed to be overkill. Perhaps it could be left in and the WARNING changed to INFO. In a release, the lib/libgcrypt-grub/ directory should be released and the lib/libgcrypt/ should probably be suppressed to avoid dependence on ruby for non-developers. import_gcry.py is python, not ruby. Yes, I mistyped that. Generally ./autogen.sh is ran before releasing tarball Yes, but it's also run by someone new (like me) who may want to contribute. The cleaner the better. -- Bruce ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Build failures on Ubuntu due to gettext
On Mon, Dec 07, 2009 at 08:14:08PM +, Carles Pina i Estany wrote:
> On Dec/07/2009, Colin Watson wrote:
> > Ubuntu's GCC enables -Wformat-security by default. This causes GCC to
> > (IMO rightly!) complain about constructs such as this:
> >
> > grub_printf (_("foo"));
>
> I see...
> (actually some weeks ago I thought about gettext security implications,
> and I thought that if someone can change the .mo files there is some
> bigger problem for the user... but I understand the point)
>
> > ... because it's all too easy for a translator to (usually
> > accidentally) insert % sequences which would cause printf to behave
>
> (side and non-relevant note: if the translator wants to do some damage on
> purpose he doesn't need to play with %s and %d, it's as easy as changing
> strings from "Press C to Cancel and D to delete" to something like (in another
> language) "Press D to Cancel and C to delete")
>
> > incorrectly. This should instead be:
> >
> > grub_printf ("%s", _("foo"));
>
> I like the general idea but I'm not 100% convinced of the
> implementation:
>
> a) It's a bit of false security because it's not fixing the case of file
> normal/menu_text.c, line 191 (search the string "Use the %C and %C keys
> to") or menu_text.c line 374 (search for "The highlighted entry")
> (I agree that it's better than before... but not very solid)
Forget security, in this case; let's just think of it as a way to avoid
translators accidentally shooting themselves (and their users) in the
foot. I have seen this happen and it's best to prevent it by static
checking.
> b) How would you translate and handle:
> grub_printf (_("Hello %s"), name);
-Wformat-security doesn't interfere with that. As its documentation
says:
At present, this warns about calls to `printf' and `scanf' functions
where the format string is not a string literal and there are no
format arguments, as in `printf (foo);'.
> c) I'm thinking to implement grub_printf_ (str). I will send a patch
> later to see if everybody likes. Then the call for simple strings would
> be:
> grub_printf_ ("%s", N_("Hello"));
>
> Not much different from:
> grub_printf_ (N_("Hello"));
>
> But it's getting hard to print a string :-)
grub_puts would be nice and would match the POSIX API (or maybe
grub_fputs or grub_putstr or something, since POSIX puts is kind of odd
in writing a trailing newline).
I don't really like significant trailing underscores in function names
myself, although I see why you chose it here.
> d) (important, even if it's the last one):
> How it prevents mistakes from the current msgfmt checks?
> For example, and using the option -c in msgfmt:
> #: normal/misc.c:67
> #, c-format
> msgid "test %s t"
> msgstr "test %d test2"
>
> /usr/bin/msgfmt -c --statistics -o po/ca.mo po/ca.po
> po/ca.po:1183: format specifications in 'msgid' and 'msgstr' for argument 1
> are
> not the same
> /usr/bin/msgfmt: found 1 fatal error
> 26 translated messages, 213 untranslated messages.
>
> Using any different number of %X from msgid and msgstr ishalting msgfmt (so,
> if
> msgid contains 1 %s and 2 %d, msgstr has to contain the same)
That only works for strings with the "c-format" attribute, which
xgettext only applies (and should only apply) to msgids that contain
"%". It helps for the string you mentioned earlier with %C in the msgid,
but it's no use for _("literal string with no percent characters").
> We are talking from _(" "), I see that -Wformat-security is for "string came
> from untrusted input and contains" when .mo are trusted. Are they?
I would recommend against considering .mo as entirely trusted, unless
you really think that developers will read every submission
line-by-line. I realise that as you say there are plenty of more subtle
avenues of attack; as I said above, I really think of this as
foot-shooting prevention more than security.
> How are other projectes implementing it? Specially the dynamic strings.
Every competent project I've seen that uses gettext in C is in line with
the patch I sent, although some use an equivalent of printf("%s", ...)
while some use an equivalent of fputs(..., stdout).
> I like the idea and I understand that it's easy to make mistakes
> translating strings. Actually I'm surprised that msgfmt is not giving
> any warning if the
In general it can only do this for appropriately tagged strings. For an
arbitrary literal string it often isn't actually wrong to translate it
with text containing a "%" sign - it's just that it crashes the program.
Thus, it's the program's responsibility to avoid crashing.
--
Colin Watson [cjwat...@ubuntu.com]
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: autogen.sh warnings
Colin Watson wrote: On Mon, Dec 07, 2009 at 11:38:06AM -0600, Bruce Dubbs wrote: configure.ac:176: required file `./config.rpath' not found The can be fixed by `touch config.rpath` configure does actually run this, so I'd recommend copying the file from gettext or gnulib instead. The gettext version is a shell script about 670 lines long that is described: # Output a system dependent set of variables, describing how to set the # run time search path of shared libraries in an executable. I'm not sure GRUB uses or needs the output of config.rpath. I was just suppressing a warning. automake: no `Makefile.am' found for any configure output I created a one line Makefile.am with the contents: SUBDIRS = . po This also fixes a warning generated by AM_GNU_GETTEXT which needed to be changed to AM_GNU_GETTEXT([external]) to avoid a complaint about a missing intl/ directory. Adding an empty ABOUT-NLS was also needed. This is likely to be delicate since we're not actually using Automake as such, and we already have a separate Makefile.in which needs to not be overwritten. I think adding a Makefile.am would make it just too easy to clobber that by accident, even if it does suppress a warning message. You are probably right. It looks like Makefile.in is basically destroyed. Not good. It looks like automake thinks it knows more than it's users and we'll have to live with the warning about missing SUBDIRS. I think I could print out a message to ignore the spurious warning. Do you want a new patch? -- Bruce ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
gettext: grub_printf_ and N_
Hi,
Attached a patch that makes a new function (grub_printf_) which one
calls _( ) for the format argument. Also adapts the current
translations but not the pending patch.
Can someone take a look? If the grub_printf_ approach is used then this
should be committed. Conflicts with Colin Watson idea because in the
other idea gettext has to be called in grub_printf level and not inside,
as far as I can think now :-)
--
Carles Pina i Estany
http://pinux.info
=== modified file 'ChangeLog'
--- ChangeLog 2009-12-07 16:46:24 +
+++ ChangeLog 2009-12-07 21:08:05 +
@@ -1,3 +1,17 @@
+2009-12-07 Carles Pina i Estany
+
+ * include/grub/misc.h (grub_printf_): New declaration.
+ * kern/misc.c (grub_printf_): New definition.
+ * normal/main.c (grub_normal_reader_init): Use `grub_printf_' and `N_'
+ instead of `grub_printf' and `_'.
+ * normal/menu_entry.c (store_completion): Likewise.
+ (run): Likewise.
+ (grub_menu_entry_run): Likewise.
+ * normal/menu_text.c (grub_wait_after_message): Likewise.
+ (notify_booting): Likewise.
+ (notify_fallback): Likewise.
+ (notify_execution_failure): Likewise.
+
2009-12-07 Colin Watson
* configure.ac: Check for vasprintf.
=== modified file 'include/grub/misc.h'
--- include/grub/misc.h 2009-12-03 23:07:29 +
+++ include/grub/misc.h 2009-12-07 21:00:46 +
@@ -171,6 +171,7 @@ char *EXPORT_FUNC(grub_strndup) (const c
void *EXPORT_FUNC(grub_memset) (void *s, int c, grub_size_t n);
grub_size_t EXPORT_FUNC(grub_strlen) (const char *s);
int EXPORT_FUNC(grub_printf) (const char *fmt, ...) __attribute__ ((format (printf, 1, 2)));
+int EXPORT_FUNC(grub_printf_) (const char *fmt, ...) __attribute__ ((format (printf, 1, 2)));
void EXPORT_FUNC(grub_real_dprintf) (const char *file,
const int line,
const char *condition,
=== modified file 'kern/misc.c'
--- kern/misc.c 2009-11-24 21:42:14 +
+++ kern/misc.c 2009-12-07 20:59:09 +
@@ -126,6 +126,19 @@ grub_printf (const char *fmt, ...)
return ret;
}
+int
+grub_printf_ (const char *fmt, ...)
+{
+ va_list ap;
+ int ret;
+
+ va_start (ap, fmt);
+ ret = grub_printf (_(fmt), ap);
+ va_end (ap);
+
+ return ret;
+}
+
#if defined (APPLE_CC) && ! defined (GRUB_UTIL)
int
grub_err_printf (const char *fmt, ...)
=== modified file 'normal/main.c'
--- normal/main.c 2009-11-25 03:48:33 +
+++ normal/main.c 2009-12-07 21:02:14 +
@@ -509,7 +509,7 @@ grub_normal_reader_init (void)
grub_normal_init_page ();
grub_setcursor (1);
- grub_printf (_("\
+ grub_printf_ (N_("\
[ Minimal BASH-like line editing is supported. For the first word, TAB\n\
lists possible command completions. Anywhere else TAB lists possible\n\
device/file completions.%s ]\n\n"),
=== modified file 'normal/menu_entry.c'
--- normal/menu_entry.c 2009-12-05 11:25:07 +
+++ normal/menu_entry.c 2009-12-07 21:02:48 +
@@ -837,7 +837,7 @@ store_completion (const char *item, grub
grub_gotoxy (0, GRUB_TERM_HEIGHT - 3);
grub_printf (" ");
- grub_printf (_("Possible %s are:"), what);
+ grub_printf_ (N_("Possible %s are:"), what);
grub_printf ("\n");
}
@@ -1000,7 +1000,7 @@ run (struct screen *screen)
grub_cls ();
grub_printf (" ");
- grub_printf (_("Booting a command list"));
+ grub_printf_ (N_("Booting a command list"));
grub_printf ("\n\n");
@@ -1182,6 +1182,6 @@ grub_menu_entry_run (grub_menu_entry_t e
grub_print_error ();
grub_errno = GRUB_ERR_NONE;
grub_putchar ('\n');
- grub_printf (_("Press any key to continue..."));
+ grub_printf_ (N_("Press any key to continue..."));
(void) grub_getkey ();
}
=== modified file 'normal/menu_text.c'
--- normal/menu_text.c 2009-12-05 11:25:07 +
+++ normal/menu_text.c 2009-12-07 21:01:41 +
@@ -40,7 +40,7 @@ void
grub_wait_after_message (void)
{
grub_putchar ('\n');
- grub_printf (_("Press any key to continue..."));
+ grub_printf_ (N_("Press any key to continue..."));
(void) grub_getkey ();
grub_putchar ('\n');
}
@@ -206,7 +206,7 @@ entry is highlighted.");
if (nested)
{
grub_printf ("\n");
- grub_printf (_("ESC to return previous menu."));
+ grub_printf_ (N_("ESC to return previous menu."));
}
}
}
@@ -627,7 +627,7 @@ notify_booting (grub_menu_entry_t entry,
void *userdata __attribute__((unused)))
{
grub_printf (" ");
- grub_printf (_("Booting \'%s\'"), entry->title);
+ grub_printf_ (N_("Booting \'%s\'"), entry->title);
grub_printf ("\n\n");
}
@@ -639,7 +639,7 @@ notify_fallback (grub_menu_entry_t entry
void *userdata __attribute__((unused)))
{
grub_printf ("\n ");
- grub_printf (_("Falling back to \'%s\'"), entry->title);
+ grub_printf_ (N_("Falling back to \'%s\'"), entry->title);
grub_printf ("\n\n");
grub_millisleep (DEFAULT_ENTRY_ERROR_DELAY_MS);
}
@@ -655,7 +655,7 @@ notify_execution_failure (void *userda
Re: gettext: normal/*
Hi, On Dec/07/2009, Jordi Mallach wrote: > On Sun, Dec 06, 2009 at 12:28:11AM +, Carles Pina i Estany wrote: > > Is it fine to have a translatable string ended with a space? I think > > that it's fine (same case than \n, even thought in this case msgfmt is > > not warning). [...] > IME, if it's not difficult to get rid of it, it better to do it for > safety, because it's a common translator mistake. I will remove the spaces. Only in one place is not straight but it's not difficult. Thanks, -- Carles Pina i Estany http://pinux.info ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Broken common.rmk change
Robert Millan wrote: > On Mon, Dec 07, 2009 at 11:50:33AM -0800, David Miller wrote: > >> From: Robert Millan >> Date: Mon, 7 Dec 2009 15:05:16 +0100 >> >> >>> On Sun, Dec 06, 2009 at 05:22:52PM -0800, David Miller wrote: >>> From: Robert Millan Date: Sun, 6 Dec 2009 14:10:26 +0100 > grub-mkconfig generates a grub.cfg that relies on UUIDs instead > of hardcoding, etc. > Well, for one thing I don't use initrd's for my Linux kernels, therefore specifying root devices via UUIDs isn't going to work. >>> I was referring to use of UUIDs internally by GRUB (e.g. the search command >>> instances grub-mkconfig generates). >>> >> Ok, but as noted in another thread this can make grub >> very slow on ieee1275 systems as we are going to look >> for the UUID on every ieee1275 device alias present and >> there can be tons of those. >> > > I'm afraid I missed that thread. In any case, we basically do/can do three > things to paliate that problem: > > - Cache the prepare_grub_to_access_device() calls at the shell/mkconfig > layer (see util/grub.d/10_linux.in for an example). > > - Disk cache (kern/disk.c). > > - Adjust disk iteration order so that disks more likely to be the ones > we're looking for show up first (e.g. on i386-pc we list hard disks > before floppies or CD). Note that UUID search aborts as soon as a > match is found; it doesn't iterate the whole list. > > More ideas welcome, of course :-) > > Seth proposed a hinting technique: search --fs_uuid ... -s root --hint hd0,1 then hd0,1 will be checked first and only on failure it will revert to scanning -- Regards Vladimir 'φ-coder/phcoder' Serbinenko signature.asc Description: OpenPGP digital signature ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Build failures on Ubuntu due to gettext
Colin Watson wrote:
> Ubuntu's GCC enables -Wformat-security by default. This causes GCC to
> (IMO rightly!) complain about constructs such as this:
>
> grub_printf (_("foo"));
>
> ... because it's all too easy for a translator to (usually accidentally)
> insert % sequences which would cause printf to behave incorrectly. This
> should instead be:
>
> grub_printf ("%s", _("foo"));
>
> Patch follows. I can't help thinking that this would be easier with a
> grub_puts, but perhaps that isn't worth it given the relatively small
> number of occurrences here?
>
> Also, should the line in notify_execution_failure instead be:
>
> - grub_printf (_("Failed to boot default entries.\n"));
> + grub_printf ("%s\n", _("Failed to boot default entries."));
>
> ... to get rid of the unsightly \n in this translated string?
>
This warning is simply wrong in this context. And silencing it is
against gettext manual. Read
http://www.gnu.org/software/hello/manual/gettext/Preparing-Strings.html
http://www.gnu.org/software/hello/manual/gettext/c_002dformat-Flag.html#c_002dformat-Flag
> 2009-12-07 Colin Watson
>
> * normal/menu_entry.c (run): Don't pass the result of gettext as
> the first argument to grub_printf, appeasing -Wformat-security.
> (grub_menu_entry_run): Likewise.
> * normal/menu_text.c (grub_wait_after_message): Likewise.
> (print_message): Likewise.
> (notify_execution_failure): Likewise.
>
> === modified file 'normal/menu_entry.c'
> --- normal/menu_entry.c 2009-12-05 11:25:07 +
> +++ normal/menu_entry.c 2009-12-07 14:02:20 +
> @@ -1000,7 +1000,7 @@ run (struct screen *screen)
>
>grub_cls ();
>grub_printf (" ");
> - grub_printf (_("Booting a command list"));
> + grub_printf ("%s", _("Booting a command list"));
>grub_printf ("\n\n");
>
>
> @@ -1182,6 +1182,6 @@ grub_menu_entry_run (grub_menu_entry_t e
>grub_print_error ();
>grub_errno = GRUB_ERR_NONE;
>grub_putchar ('\n');
> - grub_printf (_("Press any key to continue..."));
> + grub_printf ("%s", _("Press any key to continue..."));
>(void) grub_getkey ();
> }
>
> === modified file 'normal/menu_text.c'
> --- normal/menu_text.c2009-12-05 11:25:07 +
> +++ normal/menu_text.c2009-12-07 14:02:45 +
> @@ -40,7 +40,7 @@ void
> grub_wait_after_message (void)
> {
>grub_putchar ('\n');
> - grub_printf (_("Press any key to continue..."));
> + grub_printf ("%s", _("Press any key to continue..."));
>(void) grub_getkey ();
>grub_putchar ('\n');
> }
> @@ -206,7 +206,7 @@ entry is highlighted.");
>if (nested)
> {
>grub_printf ("\n");
> - grub_printf (_("ESC to return previous menu."));
> + grub_printf ("%s", _("ESC to return previous menu."));
> }
> }
> }
> @@ -655,7 +655,7 @@ notify_execution_failure (void *userdata
>grub_errno = GRUB_ERR_NONE;
> }
>grub_printf ("\n ");
> - grub_printf (_("Failed to boot default entries.\n"));
> + grub_printf ("%s", _("Failed to boot default entries.\n"));
>grub_wait_after_message ();
> }
>
>
> Thanks,
>
>
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: mkrelpath doesn't do what it should...
Am Montag, den 07.12.2009, 03:24 -0800 schrieb David Miller: > In fact I don't see why these scripts don't do that, and relatively > resolve the /boot directory whatever seperately. Because it's faster to just call once for the directory make_system_path_relative_to_its_root() instead of once for every kernel you have. Colin even added caching for prepare_grub_to_access_device() because it can be slow if you have many many kernels. -- Felix Zielcke Proud Debian Maintainer and GNU GRUB developer ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: autogen.sh warnings
Bruce Dubbs wrote: > > I'm not sure GRUB uses or needs the output of config.rpath. I was > just suppressing a warning. > Just suppressing warning is a wrong approach. If warning is there it usually means there is a reason for it. And the reason should be corrected, we shouldn't just silence the warning. It's better to leave the warning if you're unsure how to fix its reason. > I think I could print out a message to ignore the spurious warning. More output will only worse the situation. -- Regards Vladimir 'φ-coder/phcoder' Serbinenko signature.asc Description: OpenPGP digital signature ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: autogen.sh warnings
Bruce Dubbs wrote: > Vladimir 'φ-coder/phcoder' Serbinenko wrote: > >> Thanks. I'll let someone more familiar with automake to review automake >> parts. Just a comment to import_gcry > >>> These are generated by util/import_gcry.py by copying (and in some >>> cases modifying) the files in lib/libgcrypt to lib/libgcrypt-grub. It >>> also creates cipher.h, memory.h, and types.h. >>> >>> The warnings above appear to be spurious for autogen.sh > >> crypto and experimental branches have newer import_gcry.py which has >> less warning by skipping copying files unnecessary for grub2. If you >> want to fix remaining warnings (on rsa, dsa, elgamal, primegen and ecc) >> implement assymetric cryptography support (it's planned but may need >> changes to core to be useful, there are lower hanging fruits for >> crypto). > > I'm not following you completely. I am trying to clean up the build > environment, but not getting into the actual code. Some warnings need code changes to be correctly fixed. > I think the crypto was just recently merged into trunk, but I'm not > knowledgeable about the details yet to make any substantive changes. > crypto isn't really in trunk, only libgcrypt import is. The crypto is in crypto and experimental > The warning about files being copied not being modules seemed to be > overkill. Such files are actually useless. And they (useless but copied files) have to be warned about. This way we notice if new import has new feature grub2 and/or import_gcry.py should be adjusted for. In experimental only 5 files are still in this situation. > Perhaps it could be left in and the WARNING changed to INFO. This is a problem, not just INFO. -- Regards Vladimir 'φ-coder/phcoder' Serbinenko signature.asc Description: OpenPGP digital signature ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: autogen.sh warnings
On Mon, Dec 07, 2009 at 02:28:19PM -0600, Bruce Dubbs wrote: > Colin Watson wrote: >> On Mon, Dec 07, 2009 at 11:38:06AM -0600, Bruce Dubbs wrote: >>> configure.ac:176: required file `./config.rpath' not found >>> >>> The can be fixed by `touch config.rpath` >> >> configure does actually run this, so I'd recommend copying the file from >> gettext or gnulib instead. > > The gettext version is a shell script about 670 lines long that is > described: > > # Output a system dependent set of variables, describing how to set the > # run time search path of shared libraries in an executable. > > I'm not sure GRUB uses or needs the output of config.rpath. I was just > suppressing a warning. It is very likely to be incorrect to suppress a warning about a missing executable by touching an empty file. As I said, configure does use the output of config.rpath. You can verify this for yourself by searching the configure script. Whether it makes a practical difference probably depends on the platform. >> This is likely to be delicate since we're not actually using Automake as >> such, and we already have a separate Makefile.in which needs to not be >> overwritten. I think adding a Makefile.am would make it just too easy to >> clobber that by accident, even if it does suppress a warning message. > > You are probably right. It looks like Makefile.in is basically > destroyed. Not good. It looks like automake thinks it knows more than > it's users Automake's warning is pretty much correct, as it happens. GRUB is really using it in a non-recommended way, but it is not at all straightforward to either avoid using it (since the gettext build system rather likes having it) or to use it properly (which involves a complete build system revamp). > and we'll have to live with the warning about missing > SUBDIRS. I think I could print out a message to ignore the spurious > warning. > > Do you want a new patch? Seems reasonable, yes. -- Colin Watson [cjwat...@ubuntu.com] ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Build failures on Ubuntu due to gettext
Hi,
On Dec/07/2009, Colin Watson wrote:
> On Mon, Dec 07, 2009 at 08:14:08PM +, Carles Pina i Estany wrote:
> > a) It's a bit of false security because it's not fixing the case of file
> > normal/menu_text.c, line 191 (search the string "Use the %C and %C keys
> > to") or menu_text.c line 374 (search for "The highlighted entry")
> > (I agree that it's better than before... but not very solid)
>
> Forget security, in this case; let's just think of it as a way to avoid
Ok, I'll forget about security (even when the name is not helping to
forget about security :-) )
> > b) How would you translate and handle:
> > grub_printf (_("Hello %s"), name);
>
> -Wformat-security doesn't interfere with that. As its documentation
> says:
>
> At present, this warns about calls to `printf' and `scanf' functions
> where the format string is not a string literal and there are no
> format arguments, as in `printf (foo);'.
I see now (not before)
> > Using any different number of %X from msgid and msgstr ishalting
> > msgfmt (so, if msgid contains 1 %s and 2 %d, msgstr
> > has to contain the same)
>
> That only works for strings with the "c-format" attribute, which
> xgettext only applies (and should only apply) to msgids that contain
> "%". It helps for the string you mentioned earlier with %C in the msgid,
right. Actually I was expecting that all strings in a C file would
has c-format, but it's not the case:
#: normal/menu_entry.c:840
#, c-format
msgid "Possible %s are:"
#: normal/menu_entry.c:1003
msgid "Booting a command list"
And confirmed: if the translator adds %C in the second string
("Booting a command list") msgfmt -c is not complaining.
So I would apply your patch, after understanding that it's only for
strings that doesn't have any %C (and in my opinion xgettext could
add the c-format if it's in a C file, but we will not discuss it here
now :-) , I guess that in some cases would not be correct)
Your patch conflicts with my one (Subject: gettext: grub_printf_ and N_)
but it's not a big problem. If you commit before I would adapt
my one, else I would adapt your one.
--
Carles Pina i Estany
http://pinux.info
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: gettext: grub_printf_ and N_
Carles Pina i Estany wrote:
> Hi,
>
> Attached a patch that makes a new function (grub_printf_) which one
> calls _( ) for the format argument. Also adapts the current
> translations but not the pending patch.
>
> Can someone take a look? If the grub_printf_ approach is used then this
> should be committed. Conflicts with Colin Watson idea because in the
> other idea gettext has to be called in grub_printf level and not inside,
> as far as I can think now :-)
>
>
- grub_printf (_("\
+ grub_printf_ (N_("\
[ Minimal BASH-like line editing is supported. For the first word, TAB\n\
lists possible command completions. Anywhere else TAB lists possible\n\
device/file completions.%s ]\n\n"),
We already spoke about this string
- grub_printf (_("Possible %s are:"), what);
+ grub_printf_ (N_("Possible %s are:"), what);
This kind of string usage is untranslatable since when translator encounters
word "files" by itself he has no idea of its role in sentence so can't choose
apropriate case in language that have one. It has to be split into
"Possible files are:"
"Possible devices are:"
And so on
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: autogen.sh warnings
I've been looking at autogen.sh some more. Does anyone know why the lines: # FIXME: automake doesn't like that there's no Makefile.am automake -a -c -f || true are present at all? Since there is no Makefile.am, it looks like automake only creates a few files and aborts. Doing an experiment, I checked out a new version of trunk twice. In both cases I ran md5sum on all the files in the top level directory. In case 1, I didn't change anything and ran autogen.sh and then ran md5sum again. In case 2, I commented out the automake line above. As you can see below, automake only adds standard 'config.guess', 'config.sub', and 'missing' scripts. The real purpose of automake is to create a Makefile.in for configure. GRUB doesn't use it for that. Is there any reason to not just add the three files to the bzr repository and remove the automake line from autogen.sh? Along with the other changes I proposed, it would create a really clean build process. -- Bruce Case 1: > b30696711539bc73520a58a595d488d3 DISTLIST 10a12 > 7e34eb2ddac5ca0b3a0852302ef0970d aclocal.m4 11a14,17 > f9410e9a94fc672046d4997ecf1b69c9 config.guess > 55061a86b93fd314344c7f41a9662db9 config.h.in > acb6cfae431c5ce7b2319cd668c86555 config.sub > 904614dd3a3ea6442432dba9b20062df configure 25a32,34 > 0afce91a3daa1fdc32ee36370c1129dd install-sh > 6c12e662cd14b9b276338ec99c6ef3a7 md5sums > 9d9668fb32d0542b712be2c34ca79bd7 missing 26a36 > 1ded054093de910d9786c62bc4fe8cc6 stamp-h.in Case 2: 3a4 > b30696711539bc73520a58a595d488d3 DISTLIST 10a12 > 7e34eb2ddac5ca0b3a0852302ef0970d aclocal.m4 12a15,16 > 55061a86b93fd314344c7f41a9662db9 config.h.in > 904614dd3a3ea6442432dba9b20062df configure 26a31 > 29505be85d45d8a9b0a5fd3478ff6887 md5sums 27a33 > 1ded054093de910d9786c62bc4fe8cc6 stamp-h.in ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: autogen.sh warnings
Vladimir 'φ-coder/phcoder' Serbinenko wrote: Bruce Dubbs wrote: I'm not sure GRUB uses or needs the output of config.rpath. I was just suppressing a warning. Just suppressing warning is a wrong approach. If warning is there it usually means there is a reason for it. And the reason should be corrected, we shouldn't just silence the warning. It's better to leave the warning if you're unsure how to fix its reason. I agree. Please see my follow on message I sent a few minutes ago. The underlying reason for the warning is using automake for a side effect and not its intended purpose. -- Bruce ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Build failures on Ubuntu due to gettext
On Mon, Dec 07, 2009 at 11:04:52PM +0100, Vladimir 'φ-coder/phcoder' Serbinenko
wrote:
> Colin Watson wrote:
> > Ubuntu's GCC enables -Wformat-security by default. This causes GCC to
> > (IMO rightly!) complain about constructs such as this:
> >
> > grub_printf (_("foo"));
> >
> > ... because it's all too easy for a translator to (usually accidentally)
> > insert % sequences which would cause printf to behave incorrectly. This
> > should instead be:
> >
> > grub_printf ("%s", _("foo"));
> >
> > Patch follows. I can't help thinking that this would be easier with a
> > grub_puts, but perhaps that isn't worth it given the relatively small
> > number of occurrences here?
> >
> > Also, should the line in notify_execution_failure instead be:
> >
> > - grub_printf (_("Failed to boot default entries.\n"));
> > + grub_printf ("%s\n", _("Failed to boot default entries."));
> >
> > ... to get rid of the unsightly \n in this translated string?
>
> This warning is simply wrong in this context.
I disagree. I have seen many practical problems caused by this coding
style.
> And silencing it is against gettext manual. Read
> http://www.gnu.org/software/hello/manual/gettext/Preparing-Strings.html
I see nothing relevant in that link. (I am familiar with gettext
already, having been using it for many years in other free software
projects.)
> http://www.gnu.org/software/hello/manual/gettext/c_002dformat-Flag.html#c_002dformat-Flag
That suggests one available option for tweaking the way gettext handles
such strings, but it does not prescribe any particular solution to the
problem. Indeed, it says "Of course one would normally use fputs", which
is just as valid a solution as adding the c-format flag, and quite
possibly more valid since it's a heck of a lot easier to maintain. We
don't have fputs in GRUB right now, so grub_printf ("%s", _(...)) is
perfectly valid too, in just the same way that fputs is.
IME, forcing no-c-format is much more useful than forcing c-format.
In short, it is not true that "silencing it is against gettext manual",
but thank you for providing me with links that if anything support my
position. ;-)
Vladimir, please can you be a little less terse in your objections in
future? For example, it is often helpful to quote specific text rather
than vague URLs. If you would like arbitration on whether my approach is
"correct" as per the gettext maintainers, I'm happy to bring it up with
them.
Thanks,
--
Colin Watson [cjwat...@ubuntu.com]
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Build failures on Ubuntu due to gettext
Hi,
On Dec/07/2009, Vladimir '??-coder/phcoder' Serbinenko wrote:
> Colin Watson wrote:
> > Ubuntu's GCC enables -Wformat-security by default. This causes GCC to
> > (IMO rightly!) complain about constructs such as this:
> >
> > grub_printf (_("foo"));
> >
> > ... because it's all too easy for a translator to (usually accidentally)
> > insert % sequences which would cause printf to behave incorrectly. This
> > should instead be:
> >
> > grub_printf ("%s", _("foo"));
> >
> > Patch follows. I can't help thinking that this would be easier with a
> > grub_puts, but perhaps that isn't worth it given the relatively small
> > number of occurrences here?
> >
> > Also, should the line in notify_execution_failure instead be:
> >
> > - grub_printf (_("Failed to boot default entries.\n"));
> > + grub_printf ("%s\n", _("Failed to boot default entries."));
> >
> > ... to get rid of the unsightly \n in this translated string?
> >
> This warning is simply wrong in this context. And silencing it is
> against gettext manual. Read
> http://www.gnu.org/software/hello/manual/gettext/Preparing-Strings.html
> http://www.gnu.org/software/hello/manual/gettext/c_002dformat-Flag.html#c_002dformat-Flag
other solution that I like even more (but I have a problem to
implement): use --flag=_:1:pass-c-format . So all strings will be
c-format and msgfmt will check the number of parameters (even if the
string doesn't have %C, will be c-format and msgfmt should complain if
msgstr has a new %C)
--
Carles Pina i Estany
http://pinux.info
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: mkrelpath doesn't do what it should...
From: Felix Zielcke Date: Mon, 07 Dec 2009 23:06:46 +0100 > Am Montag, den 07.12.2009, 03:24 -0800 schrieb David Miller: >> In fact I don't see why these scripts don't do that, and relatively >> resolve the /boot directory whatever seperately. > > Because it's faster to just call once for the directory > make_system_path_relative_to_its_root() instead of once for every kernel > you have. > Colin even added caching for prepare_grub_to_access_device() because it > can be slow if you have many many kernels. Ok, makes sense, thanks for explaining. ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: gettext: commands/*
Carles Pina i Estany wrote:
> Hello,
>
> I've started to add gettext support in commands/*
>
> See the attached patch and ChangeLog.
>
> It's not completely exhaustive (hdparm is not in this patch).
>
> Comments before committing?
>
> Notice the the grub_register_command use N_(" ") and then these strings
> gets registered in commands/help.c when they are gonna to be on the
> screen.
>
> I've tried to translate grub_register_command and the obvious
> grub_printf. Maybe some more things need to be translated (other strings
> for example). It will be, but I would like to add gettext support to the
> 90% of the strings soon so the translators can start to work on it (and
> finish everything of course).
>
>
I've spotted some messages which may be unclear.
@@ -186,7 +187,7 @@ GRUB_MOD_INIT(boot)
{
cmd_boot =
grub_register_command ("boot", grub_cmd_boot,
- 0, "boot an operating system");
+ 0, N_("boot an operating system"));
}
This isn't space-constrained anymore. Capitalizing first letter and putting a
dot at the will make it nicer and inform translator that it's a whole sentence.
@@ -40,7 +41,7 @@ grub_cmd_cmp (grub_command_t cmd __attri
if (argc != 2)
return grub_error (GRUB_ERR_BAD_ARGUMENT, "two arguments required");
- grub_printf ("Compare `%s' and `%s':\n", args[0],
+ grub_printf (_("Compare `%s' and `%s':\n"), args[0],
args[1]);
Translator won't know if %s's are files or some other entities.
"Compare file '%s' with '%s':\n" would be an improvement. BTW here you can use
printf_ too.
@@ -49,7 +50,7 @@ grub_cmd_cmp (grub_command_t cmd __attri
goto cleanup;
if (grub_file_size (file1) != grub_file_size (file2))
-grub_printf ("Differ in size: %llu [%s], %llu [%s]\n",
+grub_printf (_("Differ in size: %llu [%s], %llu [%s]\n"),
(unsigned long long) grub_file_size (file1), args[0],
(unsigned long long) grub_file_size (file2), args[1]);
else
Same. Message is unclear.
@@ -76,7 +77,7 @@ grub_cmd_cmp (grub_command_t cmd __attri
{
if (buf1[i] != buf2[i])
{
- grub_printf ("Differ at the offset %llu: 0x%x [%s], 0x%x
[%s]\n",
+ grub_printf (_("Differ at the offset %llu: 0x%x [%s], 0x%x
[%s]\n"),
(unsigned long long) (i + pos), buf1[i], args[0],
buf2[i], args[1]);
goto cleanup;
Same
=== modified file 'commands/efi/fixvideo.c'
--- commands/efi/fixvideo.c 2009-05-04 03:49:08 +
+++ commands/efi/fixvideo.c 2009-12-06 16:39:59 +
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
static struct grub_video_patch
{
@@ -53,24 +54,24 @@ scan_card (int bus, int dev, int func, g
{
grub_target_addr_t base;
- grub_printf ("Found graphic card: %s\n", p->name);
+ grub_printf (_("Found graphic card: %s\n"), p->name);
addr += 8 + p->mmio_bar * 4;
base = grub_pci_read (addr);
if ((! base) || (base & GRUB_PCI_ADDR_SPACE_IO) ||
(base & GRUB_PCI_ADDR_MEM_PREFETCH))
- grub_printf ("Invalid MMIO bar %d\n", p->mmio_bar);
+ grub_printf (_("Invalid MMIO bar %d\n"), p->mmio_bar);
else
{
base &= GRUB_PCI_ADDR_MEM_MASK;
base += p->mmio_reg;
if (*((volatile grub_uint32_t *) base) != p->mmio_old)
- grub_printf ("Old value don't match\n");
+ grub_printf (_("Old value don't match\n"));
else
{
*((volatile grub_uint32_t *) base) = 0;
if (*((volatile grub_uint32_t *) base))
- grub_printf ("Set MMIO fails\n");
+ grub_printf (_("Set MMIO fails\n"));
}
}
@@ -79,7 +80,7 @@ scan_card (int bus, int dev, int func, g
p++;
}
- grub_printf ("Unknown graphic card: %x\n", pciid);
+ grub_printf (_("Unknown graphic card: %x\n"), pciid);
}
return 0;
This part contains incorrect English. Moreover these strings convey technical
data and I don't think they are worth translating. Actually I'll eliminate
fixvideo once we have setpci
@@ -99,7 +100,7 @@ static grub_command_t cmd_fixvideo;
GRUB_MOD_INIT(fixvideo)
{
cmd_fixvideo = grub_register_command ("fix_video", grub_cmd_fixvideo,
- 0, "Fix video problem.");
+ 0, _N("Fix video problem."));
Another unclear message. Just let this file be until we remove it.
}
=== modified file 'commands/efi/loadbios.c'
--- commands/efi/loadbios.c 2009-06-10 23:47:49 +
+++ commands/efi/loadbios.c 2009-12-06 16:43:12 +
@@ -23,6 +23,7 @@
#include
#include
Re: Build failures on Ubuntu due to gettext
Carles Pina i Estany wrote:
> And confirmed: if the translator adds %C in the second string
> ("Booting a command list") msgfmt -c is not complaining.
>
> So I would apply your patch, after understanding that it's only for
> strings that doesn't have any %C (and in my opinion xgettext could
> add the c-format if it's in a C file, but we will not discuss it here
> now :-) , I guess that in some cases would not be correct)
>
> Your patch conflicts with my one (Subject: gettext: grub_printf_ and N_)
> but it's not a big problem. If you commit before I would adapt
> my one, else I would adapt your one.
>
>
Your N_ patch superseeds Colin's patch. Feel free to use printf_ where
necessary. It solves format-security problems
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: autogen.sh warnings
Colin Watson wrote: On Mon, Dec 07, 2009 at 02:28:19PM -0600, Bruce Dubbs wrote: Colin Watson wrote: It is very likely to be incorrect to suppress a warning about a missing executable by touching an empty file. As I said, configure does use the output of config.rpath. You can verify this for yourself by searching the configure script. Whether it makes a practical difference probably depends on the platform. Two things: 1. GRUB doesn't have any config.rpath now, so configure can't use it. I only have x86 and x86_64 architectures, so it's possible that it could be installed on other platforms. 2. Further analysis shows that I induced the warning when I incorrectly created Makefile.ac, so the issue should be moot. -- Bruce ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Build failures on Ubuntu due to gettext
Hi,
On Dec/07/2009, Carles Pina i Estany wrote:
> So I would apply your patch, after understanding that it's only for
one more thing Colin. What do you think to not apply your patch and
apply the attached patch?
It force that the argument 1 of _ and N_ is a c-format (in this way
appears in grub.pot). This is, IMO, correct approach. And then msgfmt -c
checks the number of arguments.
If other projects are not using this approach they could have some
reason, like _("File") is a menu label and not a C format string. In
this cases, if this happens in Grub it's not making things worse.
Using it we should have all checks about the number of arguments and we
are not programming warning oriented.
If it's a linguism and wide accepted to do in the other way I personally
don't have any strong objection.
--
Carles Pina i Estany
http://pinux.info
=== modified file 'Makefile.in'
--- Makefile.in 2009-11-30 01:25:57 +
+++ Makefile.in 2009-12-07 23:28:49 +
@@ -477,7 +477,7 @@ genkernsyms.sh: genkernsyms.sh.in config
$(SHELL) ./config.status
$(srcdir)/po/$(PACKAGE).pot: po/POTFILES po/POTFILES-shell
- cd $(srcdir) && $(XGETTEXT) --from-code=utf-8 -o $@ -f $< --keyword=_ --keyword=N_
+ cd $(srcdir) && $(XGETTEXT) --from-code=utf-8 -o $@ -f $< --keyword=_ --keyword=N_ --flag=N_:1:c-format --flag=_:1:c-format
cd $(srcdir) && $(XGETTEXT) --from-code=utf-8 -o $@ -f po/POTFILES-shell -j --language=Shell
$(foreach lang, $(LINGUAS), $(srcdir)/po/$(lang).po): po/$(PACKAGE).pot
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Build failures on Ubuntu due to gettext
Furthermore, the style I suggested is used in many other GNU projects.
Here are just a few examples:
coreutils/src/du.c:994: error (0, 0, "%s", _("invalid zero-length file
name"));
diffutils/src/sdiff.c:853: fprintf (stderr, "%s", _("\
diffutils/src/sdiff.c-854-ed:\tEdit then use both versions, each decorated
with a header.\n\
glibc/inet/rcmd.c:178: __fxprintf(NULL, "%s",
_("\
glibc/inet/rcmd.c-179-rcmd: socket: All ports in use\n"));
gnulib/lib/xalloc-die.c:34: error (exit_failure, 0, "%s", _("memory
exhausted"));
Indeed, here's a commit from the gettext author using this style:
http://git.savannah.gnu.org/cgit/gettext.git/commit/?id=d5d1ae3b3c3ae2f837740e5f5d65326197ccdb98
(Look for close_stdout_status in lib/closeout.c.)
--
Colin Watson [cjwat...@ubuntu.com]
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: autogen.sh warnings
On Mon, Dec 07, 2009 at 04:49:08PM -0600, Bruce Dubbs wrote: > I've been looking at autogen.sh some more. > > Does anyone know why the lines: > > # FIXME: automake doesn't like that there's no Makefile.am > automake -a -c -f || true > > are present at all? Since there is no Makefile.am, it looks like > automake only creates a few files and aborts. > > Doing an experiment, I checked out a new version of trunk twice. In > both cases I ran md5sum on all the files in the top level directory. In > case 1, I didn't change anything and ran autogen.sh and then ran md5sum > again. In case 2, I commented out the automake line above. > > As you can see below, automake only adds standard 'config.guess', > 'config.sub', and 'missing' scripts. > > The real purpose of automake is to create a Makefile.in for configure. > GRUB doesn't use it for that. Is there any reason to not just add the > three files to the bzr repository and remove the automake line from > autogen.sh? These files do change from time to time, in ways that are important; for example, config.guess and config.sub are updated to support new architectures or new variants of existing architectures. It's best to have this done automatically rather than doing it once manually and then forgetting about it. We could have a temporary directory in which we run automake, just for the purpose of getting hold of these files. Perhaps something like this: rm -rf automake-tmp mkdir -p automake-tmp cat >automake-tmp/configure.ac
Re: gettext: grub_printf_ and N_
On Mon, Dec 07, 2009 at 09:11:00PM +, Carles Pina i Estany wrote:
> +int
> +grub_printf_ (const char *fmt, ...)
> +{
> + va_list ap;
> + int ret;
> +
> + va_start (ap, fmt);
> + ret = grub_printf (_(fmt), ap);
Doesn't this need to be grub_vprintf?
--
Colin Watson [cjwat...@ubuntu.com]
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Build failures on Ubuntu due to gettext
On Tue, Dec 08, 2009 at 12:25:01AM +0100, Vladimir 'φ-coder/phcoder' Serbinenko wrote: > Your N_ patch superseeds Colin's patch. Feel free to use printf_ where > necessary. It solves format-security problems It only works around the compiler warning if functions have not been given the correct function attributes. It certainly doesn't solve the underlying issue. -- Colin Watson [cjwat...@ubuntu.com] ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Build failures on Ubuntu due to gettext
On Mon, Dec 07, 2009 at 11:38:04PM +, Carles Pina i Estany wrote: > On Dec/07/2009, Carles Pina i Estany wrote: > > So I would apply your patch, after understanding that it's only for > > one more thing Colin. What do you think to not apply your patch and > apply the attached patch? > > It force that the argument 1 of _ and N_ is a c-format (in this way > appears in grub.pot). This is, IMO, correct approach. And then msgfmt -c > checks the number of arguments. I really think that this is incorrect, I'm afraid. I've certainly never seen any other project do this. There's no reason to restrict _ and N_ only to be used by printf-a-likes. Cheers, -- Colin Watson [cjwat...@ubuntu.com] ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Build failures on Ubuntu due to gettext
Colin Watson wrote:
> Furthermore, the style I suggested is used in many other GNU projects.
> Here are just a few examples:
>
> coreutils/src/du.c:994: error (0, 0, "%s", _("invalid zero-length
> file name"));
> diffutils/src/sdiff.c:853: fprintf (stderr, "%s", _("\
> diffutils/src/sdiff.c-854-ed:\tEdit then use both versions, each decorated
> with a header.\n\
> glibc/inet/rcmd.c:178: __fxprintf(NULL, "%s",
> _("\
> glibc/inet/rcmd.c-179-rcmd: socket: All ports in use\n"));
> gnulib/lib/xalloc-die.c:34: error (exit_failure, 0, "%s", _("memory
> exhausted"));
>
>
I have nothing against defining grub_putl_ (str); equivalent to
grub_printf ("%s\n", str); (defined as function if it's used extensively
for space reasons).
Just from your previous mails it seemed that you proposed to transform
strings like
grub_printf (_("Moving file %s to %s."), f1, f2); to grub_printf ("%s %s
%s %s.", _("Moving file"), f1, _("to"), f2);
to avoid translating format-strings which would be completely
untranslatable.
> Indeed, here's a commit from the gettext author using this style:
>
>
> http://git.savannah.gnu.org/cgit/gettext.git/commit/?id=d5d1ae3b3c3ae2f837740e5f5d65326197ccdb98
>
> (Look for close_stdout_status in lib/closeout.c.)
>
>
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Build failures on Ubuntu due to gettext
On Mon, Dec 07, 2009 at 10:46:30PM +, Carles Pina i Estany wrote: > Your patch conflicts with my one (Subject: gettext: grub_printf_ and N_) > but it's not a big problem. If you commit before I would adapt > my one, else I would adapt your one. Please go ahead with your patch after due discussion, and I'll adjust mine afterwards. It's too confusing to try to discuss both at the same time. -- Colin Watson [cjwat...@ubuntu.com] ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: autogen.sh warnings
Colin Watson wrote: > On Mon, Dec 07, 2009 at 04:49:08PM -0600, Bruce Dubbs wrote: > >> I've been looking at autogen.sh some more. >> >> Does anyone know why the lines: >> >> # FIXME: automake doesn't like that there's no Makefile.am >> automake -a -c -f || true >> >> are present at all? Since there is no Makefile.am, it looks like >> automake only creates a few files and aborts. >> >> Doing an experiment, I checked out a new version of trunk twice. In >> both cases I ran md5sum on all the files in the top level directory. In >> case 1, I didn't change anything and ran autogen.sh and then ran md5sum >> again. In case 2, I commented out the automake line above. >> >> As you can see below, automake only adds standard 'config.guess', >> 'config.sub', and 'missing' scripts. >> >> The real purpose of automake is to create a Makefile.in for configure. >> GRUB doesn't use it for that. Is there any reason to not just add the >> three files to the bzr repository and remove the automake line from >> autogen.sh? >> > > These files do change from time to time, in ways that are important; for > example, config.guess and config.sub are updated to support new > architectures or new variants of existing architectures. It's best to > have this done automatically rather than doing it once manually and then > forgetting about it. > > We could have a temporary directory in which we run automake, just for > the purpose of getting hold of these files. Perhaps something like this: > > rm -rf automake-tmp > mkdir -p automake-tmp > cat >automake-tmp/configure.ac < AC_INIT([temporary], [0.1]) > AC_CONFIG_AUX_DIR([.]) > AM_INIT_AUTOMAKE > AC_CANONICAL_BUILD > AC_CONFIG_FILES([Makefile]) > EOF > touch automake-tmp/Makefile.am > (cd automake-tmp && aclocal && automake -a -c -f --foreign) > cp -a automake-tmp/config.guess automake-tmp/config.sub \ > automake-tmp/install-sh automake-tmp/missing \ > . > rm -rf automake-tmp > > I don't know whether this is worth it, but it would be possible. > > Why can't Makefile.in moved to Makefile.am and then just let automake mostly copy Makefile.am to Makefile.in ? (I'm automake newbie), just an idea -- Regards Vladimir 'φ-coder/phcoder' Serbinenko signature.asc Description: OpenPGP digital signature ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: gettext: grub_printf_ and N_
Hi,
On Dec/07/2009, Colin Watson wrote:
> On Mon, Dec 07, 2009 at 09:11:00PM +, Carles Pina i Estany wrote:
> > +int
> > +grub_printf_ (const char *fmt, ...)
> > +{
> > + va_list ap;
> > + int ret;
> > +
> > + va_start (ap, fmt);
> > + ret = grub_printf (_(fmt), ap);
>
> Doesn't this need to be grub_vprintf?
right, mistake (I'm not familiar with it :-/ )
Beside calling grub_vprintf is everything all right?
--
Carles Pina i Estany
http://pinux.info
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Build failures on Ubuntu due to gettext
On Tue, Dec 08, 2009 at 12:59:28AM +0100, Vladimir 'φ-coder/phcoder' Serbinenko
wrote:
> Colin Watson wrote:
> > Furthermore, the style I suggested is used in many other GNU projects.
> > Here are just a few examples:
> >
> > coreutils/src/du.c:994: error (0, 0, "%s", _("invalid zero-length
> > file name"));
> > diffutils/src/sdiff.c:853: fprintf (stderr, "%s", _("\
> > diffutils/src/sdiff.c-854-ed:\tEdit then use both versions, each
> > decorated with a header.\n\
> > glibc/inet/rcmd.c:178: __fxprintf(NULL,
> > "%s", _("\
> > glibc/inet/rcmd.c-179-rcmd: socket: All ports in use\n"));
> > gnulib/lib/xalloc-die.c:34: error (exit_failure, 0, "%s", _("memory
> > exhausted"));
>
> I have nothing against defining grub_putl_ (str); equivalent to
> grub_printf ("%s\n", str); (defined as function if it's used extensively
> for space reasons).
> Just from your previous mails it seemed that you proposed to transform
> strings like
> grub_printf (_("Moving file %s to %s."), f1, f2); to grub_printf ("%s %s
> %s %s.", _("Moving file"), f1, _("to"), f2);
> to avoid translating format-strings which would be completely
> untranslatable.
What on earth?! I said nothing of the kind! That would obviously be
completely insane. How did you arrive at that impression?
My patch made the following transformation:
- grub_printf (_("literal string"));
+ grub_printf ("%s", _("literal string"));
This was only necessary in five places, so I doubt that a function is
worth it.
Thanks,
--
Colin Watson [cjwat...@ubuntu.com]
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: autogen.sh warnings
Vladimir 'φ-coder/phcoder' Serbinenko wrote: Why can't Makefile.in moved to Makefile.am and then just let automake mostly copy Makefile.am to Makefile.in ? (I'm automake newbie), just an idea I've been invoking automake for years, but not writing for it. I'll volunteer to try to make a proper Makefile.am. No guarantees. -- Bruce ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Build failures on Ubuntu due to gettext
Hi, On Dec/08/2009, Colin Watson wrote: > On Mon, Dec 07, 2009 at 10:46:30PM +, Carles Pina i Estany wrote: > > Your patch conflicts with my one (Subject: gettext: grub_printf_ and N_) > > but it's not a big problem. If you commit before I would adapt > > my one, else I would adapt your one. > > Please go ahead with your patch after due discussion, and I'll adjust > mine afterwards. It's too confusing to try to discuss both at the same > time. I just pushed my one since I understand that it's all right (using grub_vprintf). And it's complementary with your one. If you push your one (for which one I agree) I will adjust my pending patches to follow the coding sytle. Thanks, -- Carles Pina i Estany http://pinux.info ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Build failures on Ubuntu due to gettext
Colin Watson wrote:
> What on earth?! I said nothing of the kind! That would obviously be
> completely insane.
Sorry for having misunderstood you.
> How did you arrive at that impression?
>
Extrapolation of proposed changes since a lot of patches in gettext
threads were "samples".
> My patch made the following transformation:
>
> - grub_printf (_("literal string"));
> + grub_printf ("%s", _("literal string"));
>
> This was only necessary in five places, so I doubt that a function is
> worth it.
>
Then it's not worth it. But again we haven't gettext'ized whole grub2
yet to know for sure. Perhaps a macro so we can change it later if
necessary?
> Thanks,
>
>
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
signature.asc
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
Re: autogen.sh warnings
On Tue, Dec 08, 2009 at 01:01:28AM +0100, Vladimir 'φ-coder/phcoder' Serbinenko wrote: > Why can't Makefile.in moved to Makefile.am and then just let automake > mostly copy Makefile.am to Makefile.in ? (I'm automake newbie), just an idea I wouldn't recommend it. The syntax looks similar, but there are some slight differences, and Automake has its own ideas of what rules and variables you are and aren't allowed to override. Besides, there's quite a lot of stuff that Automake will output even with an entirely blank Makefile.am; I doubt it would be very much fun to try to merge that into the current Makefile.in without essentially doing a full port to Automake. (Now if only I had time to actually do such a port ...) -- Colin Watson [cjwat...@ubuntu.com] ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Building system (Re: autogen.sh warnings)
Colin Watson wrote: > I wouldn't recommend it. The syntax looks similar, but there are some > slight differences, and Automake has its own ideas of what rules and > variables you are and aren't allowed to override. Besides, there's quite > a lot of stuff that Automake will output even with an entirely blank > Makefile.am; I doubt it would be very much fun to try to merge that into > the current Makefile.in without essentially doing a full port to > Automake. > > (Now if only I had time to actually do such a port ...) > > You may consider this mail: http://lists.gnu.org/archive/html/grub-devel/2006-08/msg00017.html I see following variants for future developpement: - Keep ruby and improve it to address some of its deficiencies - Rewrite system in python. This way merging ruby and python dependencies. - Using automake if it's deficiencies are addressable - Using just GNU make as I proposed and implemented once. -- Regards Vladimir 'φ-coder/phcoder' Serbinenko signature.asc Description: OpenPGP digital signature ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Fixed some typos
On Mon, Dec 7, 2009 at 13:48, Colin Watson wrote: > OK. I've committed the patch at the head of this thread, then. Thanks, > Richard. Thanks for commiting to legacy :) Richard ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Building system (Re: autogen.sh warnings)
Vladimir 'φ-coder/phcoder' Serbinenko wrote: Colin Watson wrote: I wouldn't recommend it. The syntax looks similar, but there are some slight differences, and Automake has its own ideas of what rules and variables you are and aren't allowed to override. Besides, there's quite a lot of stuff that Automake will output even with an entirely blank Makefile.am; I doubt it would be very much fun to try to merge that into the current Makefile.in without essentially doing a full port to Automake. (Now if only I had time to actually do such a port ...) You may consider this mail: http://lists.gnu.org/archive/html/grub-devel/2006-08/msg00017.html I see following variants for future developpement: - Keep ruby and improve it to address some of its deficiencies - Rewrite system in python. This way merging ruby and python dependencies. - Using automake if it's deficiencies are addressable - Using just GNU make as I proposed and implemented once. That is an interesting email from 2006. It looks like this issue has been around for quite a while. What is there right now seems to be a combination of items 1, 2, and 3. I've spent some time this evening reviewing automake and autoconf and it appears that they are not appropriate for GRUB as the only build tool. For one thing, GRUB creates no libraries, but it does create a lot of specialized modules that respond in some ways like dynamic libraries, but because of the specialized nature of the boot process, have to be implemented and built in a specialized way. Using multiple tools to control the build process is difficult and error prone. Either ruby or python would do, but I would note that python is a part of the Linux Standards Base Runtime Languages specification and ruby is not. On the other hand, there is a lot less python code. There are also the shell script files to consider, however they are fairly short and most have about 12 lines of copyright info in them (there are no copyright headers in the .rmk files). The current configuration contains the following line count: 443 genmk.rb 89 conf/any-emu.rmk 634 conf/common.rmk 97 conf/gcry.rmk 206 conf/i386-coreboot.rmk 164 conf/i386-efi.rmk 145 conf/i386-ieee1275.rmk 385 conf/i386-pc.rmk 2 conf/i386-qemu.rmk 21 conf/i386.rmk 111 conf/powerpc-ieee1275.rmk 136 conf/sparc64-ieee1275.rmk 169 conf/x86_64-efi.rmk 286 util/import_gcry.py 23 genhandlerlist.sh 26 genpartmaplist.sh 47 genmodsrc.sh 77 gensymlist.sh 23 autogen.sh 75 geninit.sh 45 gendistlist.sh 27 genkernsyms.sh 4 efiemu/runtime/efiemu.sh 26 genfslist.sh 22 gencmdlist.sh 45 geninitheader.sh 19 genparttoollist.sh What autotools does offer is a standard way to control installation directories, selective tool building, and tests for prerequisite tools, but those requirements should not need to be changed frequently. A custom Makefile and/or configure with include files generated from python or ruby seems to be a reasonable approach. I'm willing to help and can use any of these tools with varying amount of initial experience. Right now, I do not have strong thoughts about the way to go, but feel that the status quo is not optimal in the long run. The above data is meant only to further discussion. -- Bruce ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: mkrelpath doesn't do what it should...
From: Felix Zielcke Date: Mon, 07 Dec 2009 11:39:22 +0100 > The one mount point case is now fixed in r1917 > See Subject: `handling mount points in grub-mkrelpath' from me for the > patch. Seems to work properly now. The paths all start with "//" but I guess that's ok. Thanks! ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Broken common.rmk change
From: Robert Millan Date: Mon, 7 Dec 2009 15:04:11 +0100 > On Sun, Dec 06, 2009 at 04:21:50PM -0800, David Miller wrote: >> From: Robert Millan >> Date: Sun, 6 Dec 2009 14:10:26 +0100 >> >> > We're actually in the process of getting rid of device.map; >> >> Meanwhile, please fix the regression you added to the tree. :-) > > I think this should do it. Please can you test? This patch works fine for me, thanks! ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
