Re: [PATCH] switch from sprintf to asprintf and snprintf

[Colin Watson]

On Tue, Dec 29, 2009 at 10:30:12AM +0100, Vladimir 'φ-coder/phcoder' Serbinenko 
wrote:
> +char *EXPORT_FUNC(grub_asprintf) (const char *fmt, ...)
> + __attribute__ ((format (printf, 1, 2)));

It's very confusing that you've made grub_asprintf have a dramatically
different interface from asprintf. Perhaps you could call this
grub_xasprintf instead? (Although I notice that it doesn't die when
malloc fails, but merely returns NULL.)

> +char *EXPORT_FUNC(grub_avsprintf) (const char *fmt, va_list args);

The conventional spelling is vasprintf, not avsprintf.

-- 
Colin Watson   [cjwat...@ubuntu.com]


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

Re: [RFC] Multiboot ammendment: non-VBE video

[Robert Millan]

On Mon, Dec 28, 2009 at 01:07:10PM +0100, Vladimir 'φ-coder/phcoder' Serbinenko 
wrote:
> Robert Millan wrote:
> > On Tue, Sep 01, 2009 at 05:37:11PM +0200, Vladimir 'phcoder' Serbinenko 
> > wrote:
> >   
> >> Hello. I'm implementing video part of multiboot specification.
> >> Currently the only defined interface is for providing VBE info. I
> >> propose following way to set fields if video is non VBE:
> >> vbe_control_info=0x
> >> When vbe_control_info is set to 0x all VBE-specific fields are 
> >> invalid
> >> vbe_mode set to 0x
> >> vbe_interface_seg=0x
> >> vbe_interface_off=0x
> >> vbe_interface_len=0xff
> >> vbe_mode_info points to structure similar to vbe_mode_info but with
> >> all vbe-specific fields set to zero. Remaining (valid) fields are
> >> (full structur is in include/grub/i386/pc/vbe.h)
> >> 
> >
> > This seems like one of these cases in which legacy gets in the way.  Why
> > don't we just replace the legacy structure with something that doesn't need
> > dummy fields?
> >   
> Actually it seems like we already define possible dummy fields if
> vbe_interface isn't available.

Yes.  That's the case for vbe_interface_* fields.  But extending this to
vbe_control_info and vbe_mode_info is backward-incompatible.

I don't object to backward-incompatible changes, but I would expect that
when we make one, we get the benefit of a clean, legacy-free interface.  In
this case we carry on with legacy baggage to archieve half-way backward
compatibility.

In purely practical terms, all of this only has a minimal effect.  GRUB Legacy
didn't implement these extensions, and GRUB 2 hasn't yet, so use of this
feature in loadees isn't widespread.  But it has a major impact when it comes
to reputation.  If we make incompatible changes in the text, we should be
honest about it.  I don't want people to think they can't rely on Multiboot
because its maintainers sometimes stretch the definitions in ways not
permitted by the text.

Since it's obvious we want to change something (rather than implement
VBE-specific extensions), I propose we start with:

-If bit 11 in the @samp{flags} is set, the graphics table is available.
+If bit 12 in the @samp{flags} is set, the graphics table is available.

at this point, we can change anything we like.

-- 
Robert Millan

  "Be the change you want to see in the world" -- Gandhi


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

Re: autogen.sh warnings

[Robert Millan]

On Mon, Dec 28, 2009 at 09:50:47PM -0600, Bruce Dubbs wrote:
> Robert Millan wrote:
>>
>> What is exactly the problem?  
>
> Using automake without Makefile.am is non-standard and not provided for  
> within automake.  The only thing we use automake for is to copy  
> config.{guess,sub} to the root of the root of the source.
>
> Also, building as one large monolithic Makefile with includes built via  
> scripts is probably not optimal from a comprehension point of view.

That's a long-standing problem, with no easy solution.  But as for automake,
I don't think it'd be a bad idea to migrate Makefile.in to Makefile.am.  We
already have kludges in Makefile.in (e.g. docs/version.texi generation) which
would completely disappear if this file was automake'd.

Any takers?

-- 
Robert Millan

  "Be the change you want to see in the world" -- Gandhi


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

Re: [PATCH] grub-probe support for NetBSD

[Robert Millan]

On Tue, Dec 29, 2009 at 02:31:46AM +0100, Grégoire Sutre wrote:
> +#if defined(__NetBSD__)
> +   /* Convert this block device to its character (raw) device */
> +   res = xmalloc (strlen (cwd) + strlen (ent->d_name) + 3);
> +   sprintf (res, "%s/r%s", cwd, ent->d_name);
> +#else
> res = xmalloc (strlen (cwd) + strlen (ent->d_name) + 2);
> sprintf (res, "%s/%s", cwd, ent->d_name);
> +#endif

Can you avoid code duplication here?  Something like:

#ifdef __NetBSD__
  const char *template = "%s/r%s";
#else
  const char *template = "%s/%s";
#endif

-- 
Robert Millan

  "Be the change you want to see in the world" -- Gandhi


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

Re: [PATCH] switch from sprintf to asprintf and snprintf

[Robert Millan]

On Tue, Dec 29, 2009 at 10:30:12AM +0100, Vladimir 'φ-coder/phcoder' Serbinenko 
wrote:
> sprintf is potentially dangerous especially with gettext, when messages
> may be larger than coder would expect. I attach the patch to fix it

Could you split the patches into one for asprintf and one for *nprintf?  The
asprintf one is something I'd really like to see in trunk.  For the rest I'm
not so sure.

-- 
Robert Millan

  "Be the change you want to see in the world" -- Gandhi


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

Re: [PATCH] switch from sprintf to asprintf and snprintf

[Robert Millan]

On Fri, Jan 01, 2010 at 09:32:24AM +, Colin Watson wrote:
> On Tue, Dec 29, 2009 at 10:30:12AM +0100, Vladimir 'φ-coder/phcoder' 
> Serbinenko wrote:
> > +char *EXPORT_FUNC(grub_asprintf) (const char *fmt, ...)
> > + __attribute__ ((format (printf, 1, 2)));
> 
> It's very confusing that you've made grub_asprintf have a dramatically
> different interface from asprintf. Perhaps you could call this
> grub_xasprintf instead?

Is it feasible to implement the same interface as asprintf instead?

-- 
Robert Millan

  "Be the change you want to see in the world" -- Gandhi


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

Re: [PATCH] compilation with external intl library

[Robert Millan]

On Wed, Dec 30, 2009 at 11:23:38AM +0100, Yves Blusseau wrote:
> Hi,
> 
> this is the patch to "fix" compilation with external intl library. It fix the 
> #28356 bug (http://savannah.gnu.org/bugs/?28356).
> 
> I follow strictly the guideline of gettext, and now we can compile grub with 
> external intl library.
> 
> Some enhancements;
>   - We can disable NLS (Native Language Support) with the --disable-nls 
> configure flag.
>   - We can choose where the external intl library is with the 
> --with-libintl-prefix configure flag.
> 
> Under linux the gettext macro detect that the gettext function is in libc and 
> so don't add any flags (like before).
> On other platform like Mac OSX, the macro add the proper compile and link 
> flags (ex: -lintl -liconv -lc  -Wl,-framework -Wl,CoreFoundation in OSX)
> 
> Best Regards,
> Yves Blusseau
> 
> PS: if it ok i'll commit this patch and will add another one to make on error 
> if the intl library can't be use and show alternative to compile grub.

Hi Yves,

Please can you adjust your mailer so that it marks text attachments as
text/plain?  It currently sets Content-Type: application/octet-stream which
makes it hard to send a context-reply with my mailer.

I had other comments, but in particular, config.rpath shouldn't be imported
statically.

Thanks!

-- 
Robert Millan

  "Be the change you want to see in the world" -- Gandhi


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

Re: [PATCH] switch from sprintf to asprintf and snprintf

[Colin Watson]

On Fri, Jan 01, 2010 at 12:52:56PM +0100, Robert Millan wrote:
> On Fri, Jan 01, 2010 at 09:32:24AM +, Colin Watson wrote:
> > On Tue, Dec 29, 2009 at 10:30:12AM +0100, Vladimir 'φ-coder/phcoder' 
> > Serbinenko wrote:
> > > +char *EXPORT_FUNC(grub_asprintf) (const char *fmt, ...)
> > > + __attribute__ ((format (printf, 1, 2)));
> > 
> > It's very confusing that you've made grub_asprintf have a dramatically
> > different interface from asprintf. Perhaps you could call this
> > grub_xasprintf instead?
> 
> Is it feasible to implement the same interface as asprintf instead?

asprintf is not really the ideal interface; most people forget to handle
errors when using it, and it's verbose even when used well. If there's a
single approach to out-of-memory errors that's appropriate throughout,
then something like xasprintf is much better.

-- 
Colin Watson   [cjwat...@ubuntu.com]


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

Re: Config file generation failure on Linux

[Robert Millan]

On Wed, Dec 30, 2009 at 04:01:01PM +0100, Grégoire Sutre wrote:
> Hi,
>
> Config file generation with grub-mkconfig fails on Linux when grub is  
> not installed in the same prefix as gettext, since the script  
> util/grub.d/10_linux.in contains:
>
> . ${bindir}/gettext.sh
>
> A possible patch is given below, but there are surely other ways to deal  
> with it.  The patch simply falls back to echo instead of gettext if the  
> gettext binary cannot be found.

It'd be better if this was an autoconf check.

-- 
Robert Millan

  "Be the change you want to see in the world" -- Gandhi


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

Re: [PATCH 2/3] Compilation with external intl library

[Robert Millan]

On Thu, Dec 31, 2009 at 05:55:04PM +0100, Yves Blusseau wrote:
> +/* Disabled NLS.
> +   The casts to 'const char *' serve the purpose of producing warnings
> +   for invalid uses of the value returned from these functions.
> +   On pre-ANSI systems without 'const', the config.h file is supposed to
> +   contain "#define const".  */
> +# define gettext(Msgid) ((const char *) (Msgid))
> +# define grub_gettext(str) ((const char *) (str))

This should have #ifdef GRUB_UTIL / #else.

> +  grub_util_init_nls();

Please add a space in-between (in this and other calls).

> diff --git a/util/mkisofs/mkisofs.h b/util/mkisofs/mkisofs.h
> index 79ae502..55b4aa6 100644
> --- a/util/mkisofs/mkisofs.h
> +++ b/util/mkisofs/mkisofs.h
> @@ -30,10 +30,7 @@
>  #include 
>  #include 
>  
> -#include 
> -#include 
> -#define _(str) gettext(str)
> -#define N_(str) str
> +#include 

Please don't make mkisofs depend on GRUB headers.  It's a bit confusing,
but mkisofs is supposed to be a separate package.

-- 
Robert Millan

  "Be the change you want to see in the world" -- Gandhi


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

Re: [PATCH 3/3] Compilation with external intl library

[Robert Millan]

On Thu, Dec 31, 2009 at 05:55:09PM +0100, Yves Blusseau wrote:
> 2009-12-31  Yves Blusseau  
> 
>   * util/mkisofs/mkisofs.c: fix a warning about a bad cast when NLS
>   is disabled
> ---
>  ChangeLog.bad-cast |6 ++
>  util/mkisofs/mkisofs.c |2 +-
>  2 files changed, 7 insertions(+), 1 deletions(-)
>  create mode 100644 ChangeLog.bad-cast

> diff --git a/ChangeLog.bad-cast b/ChangeLog.bad-cast
> new file mode 100644
> index 000..97ee16b
> --- /dev/null
> +++ b/ChangeLog.bad-cast
> @@ -0,0 +1,6 @@
> +Compilation with external intl library
> +
> +2009-12-31  Yves Blusseau  
> +
> + * util/mkisofs/mkisofs.c: fix a warning about a bad cast when NLS
> + is disabled
> diff --git a/util/mkisofs/mkisofs.c b/util/mkisofs/mkisofs.c
> index 4ed091b..57e77dd 100644
> --- a/util/mkisofs/mkisofs.c
> +++ b/util/mkisofs/mkisofs.c
> @@ -490,7 +490,7 @@ void usage(){
> int comma;
> int len;
> unsigned int j;
> -   char *arg;
> +   const char *arg;
>  
> printf ("  ");

This is fine.  In the future, feel free to commit this kind of obvious
cleanup directly.

Thanks!

-- 
Robert Millan

  "Be the change you want to see in the world" -- Gandhi


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

Re: [PATCH] macrofy (multiterm branch)

[Robert Millan]

On Thu, Dec 31, 2009 at 07:31:56PM +0100, Carles Pina i Estany wrote:
> 
> Hi,
> 
> Find an attached patch to macrofy pos >> 8 and pos & 0xff in multiter.
> In my opinion should be macrofied, it's more clear.

Agreed.

> === modified file 'include/grub/term.h'
> --- include/grub/term.h   2009-12-25 02:37:20 +
> +++ include/grub/term.h   2009-12-30 18:17:55 +
> @@ -32,6 +32,8 @@
>  #define GRUB_TERM_ESC'\e'
>  #define GRUB_TERM_TAB'\t'
>  #define GRUB_TERM_BACKSPACE  8
> +#define GETX(XY) (XY >> 8)
> +#define GETY(XY) (XY & 0xff)

It should have GRUB_ prefix.  Otherwise it can go in.

-- 
Robert Millan

  "Be the change you want to see in the world" -- Gandhi


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

Re: grub.info

[Robert Millan]

On Thu, Dec 31, 2009 at 01:04:47PM -0500, Bruce O. Benson wrote:
> 2009/12/31 Vladimir 'φ-coder/phcoder' Serbinenko 
> 
> > Bruce O. Benson wrote:
> > >
> > > 2009/12/30 Bruce Dubbs  > > >
> > >
> > > Bruce O. Benson wrote:
> > > All contributors can safely, legally add/merge/move stuff there if
> > > desired, delete anything/everything I wrote, add attributions I could
> > > not find, etc.
> > >
> > It's not how it works. For the documentation to be accepted to GNU
> > project contributors have to sign legal papers.
> > Now I'm going to comment on value of manual:
> > "Installation via Package Management" is distribution-specific so it's
> > inappropriate for upstream manual
> > "Building / Installation from Source" is a part of INSTALL and we
> > already have this
> > "Utilities" - description of tools is inappropriate. User doesn't care
> > if it's ELF or text file but how to use them.
> > "Pre-Boot" - is wrong. MBR doesn't point anywhere it's executable code.
> > "grub-rescue" part is wrong too
> > "Lua shell" is part of grub-extras, not grub itself.
> > In whole the manual is vague, contains inappropriate or inexact
> > information.
> > If we merge any part of this manual into bzr we would need to proofread
> > every patch
> >
> 
> This is the typical problem of contributing complaints vs. code.

Bruce, there's absolutely no problem with Vladimir helping with review of
code, documentation and proposals in general.  In fact, I find this very
helpful.

You too are welcome to provide your feedback and participate in the
discussions.  If you do this, I will appreciate that you stay on topic
and respectful to others.

Thank you

-- 
Robert Millan

  "Be the change you want to see in the world" -- Gandhi


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

Re: [PATCH] spaces (label, UUID)

[Robert Millan]

On Mon, Dec 28, 2009 at 02:17:37AM +0100, Carles Pina i Estany wrote:
> === modified file 'normal/misc.c'
> --- normal/misc.c 2009-12-20 23:32:15 +
> +++ normal/misc.c 2009-12-28 01:15:34 +
> @@ -68,7 +68,10 @@ grub_normal_print_device_info (const cha
> if (grub_errno == GRUB_ERR_NONE)
>   {
> if (label && grub_strlen (label))
> - grub_printf_ (N_("- Label %s"), label);
> + {
> +   grub_putchar (' ');
> +   grub_printf_ (N_("- Label \"%s\""), label);
> + }
> grub_free (label);
>   }
> grub_errno = GRUB_ERR_NONE;
> @@ -81,6 +84,7 @@ grub_normal_print_device_info (const cha
> if (grub_errno == GRUB_ERR_NONE)
>   {
> grub_unixtime2datetime (tm, &datetime);
> +   grub_putchar (' ');
> grub_printf_ (N_("- Last modification time %d-%02d-%02d "
>  "%02d:%02d:%02d %s"),
>  datetime.year, datetime.month, datetime.day,

Why not just add a space before the -?

Uhm, I guess I know the answer in advance :-)

Carles, we shouldn't avoid changing translatable strings at the cost of
making the code bigger or less efficient.  It's not hard for translators
to reuse the old string.

I notice that you committed it already.  Please can you adjust?

-- 
Robert Millan

  "Be the change you want to see in the world" -- Gandhi


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

Re: grub-setup: error: no mapping exists for ... in GRUB2 v1.97.1 on fake (IMSM) RAID

[Robert Millan]

On Sun, Dec 27, 2009 at 05:50:35PM -0800, Lapohos Tibor wrote:
> > grub-install --modules=raid /dev/md126

You're not supposed to force addition of raid.mod.  If it wasn't auto-detected,
it means there is a problem somewhere.  Working around it just makes GRUB fail
later on in a more unpleasant way ;-)

-- 
Robert Millan

  "Be the change you want to see in the world" -- Gandhi


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

Re: Compilation under MacOSX

[Robert Millan]

On Tue, Dec 29, 2009 at 04:29:48PM +0100, Yves Blusseau wrote:
> gcc-4.2 -o grub-mkpasswd-pbkdf2 grub_mkpasswd_pbkdf2-gnulib_progname.o 
> grub_mkpasswd_pbkdf2-util_grub_mkpasswd_pbkdf2.o 
> grub_mkpasswd_pbkdf2-lib_crypto.o 
> grub_mkpasswd_pbkdf2-lib_libgcrypt_grub_cipher_sha512.o 
> grub_mkpasswd_pbkdf2-lib_pbkdf2.o grub_mkpasswd_pbkdf2-util_misc.o 
> grub_mkpasswd_pbkdf2-kern_err.o -L/opt/local/lib -lintl 
> Undefined symbols:
>   "_getline", referenced from:
>   _main in grub_mkpasswd_pbkdf2-util_grub_mkpasswd_pbkdf2.o
>   _main in grub_mkpasswd_pbkdf2-util_grub_mkpasswd_pbkdf2.o
> ld: symbol(s) not found
> collect2: ld returned 1 exit status
> make: *** [grub-mkpasswd-pbkdf2] Error 1
> 
> seems that the getline function is not defined anywhere.

I just imported getline() from Gnulib.

-- 
Robert Millan

  "Be the change you want to see in the world" -- Gandhi


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

Re: [PATCH] compilation with external intl library

[Robert Millan]

On Fri, Jan 01, 2010 at 01:00:10PM +0100, Robert Millan wrote:
> 
> Please can you adjust your mailer so that it marks text attachments as
> text/plain?  It currently sets Content-Type: application/octet-stream which
> makes it hard to send a context-reply with my mailer.

Oh, I notice you already did!  Sorry for bothering :-)

-- 
Robert Millan

  "Be the change you want to see in the world" -- Gandhi


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

Re: [PATCH] Proof of concept interrupt wrapping

[Robert Millan]

On Thu, Dec 31, 2009 at 01:28:30PM +0100, Vladimir 'φ-coder/phcoder' Serbinenko 
wrote:
> Hello. We were discussing with Robert how to move BIOS interrupt
> routines out of kernel. There are following possibilities:
> 1) Have a .lowmem section in every concerned module which will always be
> placed in low memory. Currently in experimental.
> Advantages:
>   a) moving functions to modules is straightforward
>   b) functions grow neither in size nor in complexity
> Disadvantages:
>   c) needs lowmem allocators in core
> 2) Make every function needing bios interrupts setup its own trampoline.
> Due to complexity of trampolines it's not a real option
> 3) Have an universal routine grub_interrupt (int intno, struct
> grub_cpu_interrupt_regs *regs) which will be used by C routine to do the
> interrupt calls. This would move the complexity from asm to C.
> Advantages:
> a) simplicity in core
> b) complexity moved to a more readable language
> c)  we can also rename grub_interrupt to grub_interrupt_real and make
> grub_interrupt dprintf registers before and after the call. This would
> make debugging BIOS quirks easier.
> Disadvantages:
> a) Moving functions needs effort
> b) C functions are probably bigger but it may be offset by possibility
> of inlining functions
> c) repeadetly changing from/to real mode is an overhead when executing
> multiple interrupts in series. Fortunately this condition is rare in our
> codebase and is only on non performance-critical parts like halting.
> d) Some functions aren't covered by this. At least grub_pxe_call is in
> this case. But we can use method 2 for them

We could diminish #1.c with ifdef GRUB_MACHINE_PCBIOS, but it's still ugly.
I like #3 a lot more.

As for C being bigger than asm, it's argueable, taking into account that
we have regparm, function alignment hacks, and gcc size optimizations :-)

In any case #3 looks a lot cleaner.

Some comments about the patch:

> +struct grub_cpu_int_registers
> +{
> +  grub_uint16_t bx;
> +  grub_uint16_t es;
> +  grub_uint16_t cx;
> +  grub_uint16_t ax;
> +  grub_uint16_t dx;
> +  grub_uint16_t ds;
> +  grub_uint16_t di;
> +  grub_uint16_t flags;
> +  grub_uint16_t si;
> +};

This structure is named in generic way, but its member names are CPU-specific.
Is it useful to make this generic?  In practice, it will be impossible for
CPU-independant code to use it.

Besides, it's biased towards BIOS as the only i386 way.  If, say, some new
i386 firmware provides an interrupt-based callback interface that we're
compelled to support (and YES I really hope this won't happen...), it
wouldn't use i8086 mode at all.

I think it would be better to admit that this is a pure ad-hoc kludge.  It's
not beautiful, but it's *much* better than what we have now.

-- 
Robert Millan

  "Be the change you want to see in the world" -- Gandhi


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

Re: Regarding virtual to physical memory mapping..

[Robert Millan]

On Thu, Dec 31, 2009 at 01:31:05PM +0100, Vladimir 'φ-coder/phcoder' Serbinenko 
wrote:
> > I wanted to know whether I can allocate some
> > memory in the region below 1MB?
> Only in experimental branch. But we're not currently sure if we will use
> this.

We can use this.  My concerns are with making intrusive changes in the
standard heap.  I think it's important that kern/mm.c is small and simple,
but this doesn't preclude doing more fancy stuff in non-core areas.

Sorry if I was unclear about it.

-- 
Robert Millan

  "Be the change you want to see in the world" -- Gandhi


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

Re: [PATCH] spaces (label, UUID)

[Carles Pina i Estany]

Hi,

On Jan/01/2010, Robert Millan wrote:
> On Mon, Dec 28, 2009 at 02:17:37AM +0100, Carles Pina i Estany wrote:
> > === modified file 'normal/misc.c'
> > --- normal/misc.c   2009-12-20 23:32:15 +
> > +++ normal/misc.c   2009-12-28 01:15:34 +
> > @@ -68,7 +68,10 @@ grub_normal_print_device_info (const cha
> >   if (grub_errno == GRUB_ERR_NONE)
> > {
> >   if (label && grub_strlen (label))
> > -   grub_printf_ (N_("- Label %s"), label);
> > +   {
> > + grub_putchar (' ');
> > + grub_printf_ (N_("- Label \"%s\""), label);
> > +   }

> Why not just add a space before the -?
> 
> Uhm, I guess I know the answer in advance :-)

I think that you thought about half of the reasons :-)

> Carles, we shouldn't avoid changing translatable strings at the cost
> of making the code bigger or less efficient.  It's not hard for
> translators to reuse the old string.

This is half of the reason (and not too bad: this translation will
appear as "fuzzy" so it's not everything lost)

The other reason:
http://lists.gnu.org/archive/html/grub-devel/2009-12/msg00131.html

Avoiding to have trailing spaces helps the translators (they will do
less mistakes).

Of course makes the code slightly bigger (source code and binary)

> I notice that you committed it already.  Please can you adjust?

Yes I can adjust, but could you reconfirm if I should adjust after above
message?

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: Year 2010

[Alfred M. Szmidt]

   Now that we changed year, the usual mess with Copyright lines
   strikes again.

   Remember that when committing copyright-significant changes,
   copyright lines should be updated.  I'll review existing files for
   all the updates that could have been missed during 2009.

   Please keep in mind to add 2010 to the list where appropiate!

The current practise is to update all files at the start of the new
year; not just the files one modifies during the year.  A useful way
of doing it is to use gnulib's update-copyright module.



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

Re: Year 2010

[Robert Millan]

On Fri, Jan 01, 2010 at 10:38:19AM -0500, Alfred M. Szmidt wrote:
>Now that we changed year, the usual mess with Copyright lines
>strikes again.
> 
>Remember that when committing copyright-significant changes,
>copyright lines should be updated.  I'll review existing files for
>all the updates that could have been missed during 2009.
> 
>Please keep in mind to add 2010 to the list where appropiate!
> 
> The current practise is to update all files at the start of the new
> year; not just the files one modifies during the year.  A useful way
> of doing it is to use gnulib's update-copyright module.

Hi Alfred,

Since it's not a lot of effort, I prefer to strive for correctness.  But
thanks for the tip :-)

-- 
Robert Millan

  "Be the change you want to see in the world" -- Gandhi


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

Re: [PATCH] spaces (label, UUID)

[Robert Millan]

On Fri, Jan 01, 2010 at 02:10:51PM +0100, Carles Pina i Estany wrote:
> 
> Hi,
> 
> On Jan/01/2010, Robert Millan wrote:
> > On Mon, Dec 28, 2009 at 02:17:37AM +0100, Carles Pina i Estany wrote:
> > > === modified file 'normal/misc.c'
> > > --- normal/misc.c 2009-12-20 23:32:15 +
> > > +++ normal/misc.c 2009-12-28 01:15:34 +
> > > @@ -68,7 +68,10 @@ grub_normal_print_device_info (const cha
> > > if (grub_errno == GRUB_ERR_NONE)
> > >   {
> > > if (label && grub_strlen (label))
> > > - grub_printf_ (N_("- Label %s"), label);
> > > + {
> > > +   grub_putchar (' ');
> > > +   grub_printf_ (N_("- Label \"%s\""), label);
> > > + }
> 
> > Why not just add a space before the -?
> > 
> > Uhm, I guess I know the answer in advance :-)
> 
> I think that you thought about half of the reasons :-)
> 
> > Carles, we shouldn't avoid changing translatable strings at the cost
> > of making the code bigger or less efficient.  It's not hard for
> > translators to reuse the old string.
> 
> This is half of the reason (and not too bad: this translation will
> appear as "fuzzy" so it's not everything lost)
> 
> The other reason:
> http://lists.gnu.org/archive/html/grub-devel/2009-12/msg00131.html
> 
> Avoiding to have trailing spaces helps the translators (they will do
> less mistakes).
> 
> Of course makes the code slightly bigger (source code and binary)

Meh, ok.  I hope we don't have a lot of these...

-- 
Robert Millan

  "Be the change you want to see in the world" -- Gandhi


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

Re: autogen.sh warnings

[Bruce Dubbs]

Robert Millan wrote:

On Mon, Dec 28, 2009 at 09:50:47PM -0600, Bruce Dubbs wrote:

Robert Millan wrote:
What is exactly the problem?  
Using automake without Makefile.am is non-standard and not provided for  
within automake.  The only thing we use automake for is to copy  
config.{guess,sub} to the root of the root of the source.


Also, building as one large monolithic Makefile with includes built via  
scripts is probably not optimal from a comprehension point of view.


That's a long-standing problem, with no easy solution. 


That's for sure.


But as for automake,
I don't think it'd be a bad idea to migrate Makefile.in to Makefile.am.  We
already have kludges in Makefile.in (e.g. docs/version.texi generation) which
would completely disappear if this file was automake'd.

Any takers?


I thought about it, but I really don't have much experience writing for 
autotools.  AFAICT, it would require getting rid of all the ruby and 
gen*.sh scripts and generally be very invasive.


As you know, GRUB supports many OSes, file systems, and BIOSes.  The 
nature of the process is closer to an operating system than a standard 
program.  The more I look at it, the more impressed I am that you guys 
get as much as you do working.


I think it would take many iterations to get an autotooled build system 
right.


  -- Bruce


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

[PATCH] help command fix (II)

[Carles Pina i Estany]

Hello,

Some days ago I sent:
http://lists.gnu.org/archive/html/grub-devel/2009-12/msg00433.html

As Vladimir commented on IRC: patch was not checking how long the
strings appears on the screen (multiple-byte chars, etc.)

Find attached a new version, measuring the width of the strings. I have
tested it adding some long summary in some commands and works fine
(apparently at least).

Thank you,

-- 
Carles Pina i Estany
http://pinux.info
=== modified file 'ChangeLog'
--- ChangeLog	2010-01-01 12:33:45 +
+++ ChangeLog	2010-01-01 17:08:20 +
@@ -1,3 +1,14 @@
+2010-01-01  Carles Pina i Estany  
+
+	* commands/help.c: Include `grub/mm.h' and `grub/normal.h'.
+	(grub_cmd_help): Print the cmd->name before the cmd->summary. Cut the
+	string using string width.
+	* normal/menu_text.c (grub_print_message_indented): Use
+	grub_print_spaces and not print_spaces.
+	(print_timeout): Likewise.
+	(print_spaces): Move to...
+	* include/grub/term.h: ... here. Change the name to grub_print_spaces.
+
 2010-01-01  Robert Millan  
 
 	Import from Gnulib.

=== modified file 'commands/help.c'
--- commands/help.c	2009-12-25 23:50:59 +
+++ commands/help.c	2010-01-01 17:06:21 +
@@ -22,6 +22,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 static grub_err_t
 grub_cmd_help (grub_extcmd_t ext __attribute__ ((unused)), int argc,
@@ -38,19 +40,42 @@ grub_cmd_help (grub_extcmd_t ext __attri
   if ((cmd->prio & GRUB_PRIO_LIST_FLAG_ACTIVE) &&
 	  (cmd->flags & GRUB_COMMAND_FLAG_CMDLINE))
 	{
-	  char description[GRUB_TERM_WIDTH / 2];
-	  const char* summary_translated = _(cmd->summary);
-	  int desclen = grub_strlen (summary_translated);
-
-	  /* Make a string with a length of GRUB_TERM_WIDTH / 2 - 1 filled
-	 with the description followed by spaces.  */
-	  grub_memset (description, ' ', GRUB_TERM_WIDTH / 2 - 1);
-	  description[GRUB_TERM_WIDTH / 2 - 1] = '\0';
-	  grub_memcpy (description, summary_translated,
-		   (desclen < GRUB_TERM_WIDTH / 2 - 1
-			? desclen : GRUB_TERM_WIDTH / 2 - 1));
+	  char *command_help;
+	  const char *summary_translated = _(cmd->summary);
+	  grub_uint32_t *unicode_command_help;
+	  grub_uint32_t *unicode_last_position;
+
+	  command_help = grub_malloc (grub_strlen (cmd->name) +
+	  			  sizeof (" ") - 1 +
+  grub_strlen (summary_translated));
+	  			  
+	  grub_sprintf(command_help, "%s %s", cmd->name, summary_translated);
+
+	  grub_utf8_to_ucs4_alloc (command_help, &unicode_command_help,
+	  			   &unicode_last_position);
+
+	  while (grub_getstringwidth (unicode_command_help,
+	  			  unicode_last_position) > 
+  		(GRUB_TERM_WIDTH / 2) - 2)
+	{
+	  unicode_last_position--;
+	}
 
-	  grub_printf ("%s%s", description, (cnt++) % 2 ? "\n" : " ");
+	  grub_print_ucs4 (unicode_command_help, unicode_last_position);
+	  
+	  if ((cnt++) % 2)
+	{
+	  grub_putchar ('\n');
+	}
+	else
+	{
+	  grub_print_spaces (GRUB_TERM_WIDTH / 2 - 
+	  			 grub_getstringwidth (unicode_command_help,
+ 		  unicode_last_position));
+	}
+	  
+	  grub_free (command_help);
+	  grub_free (unicode_command_help);
 	}
   return 0;
 }

=== modified file 'include/grub/term.h'
--- include/grub/term.h	2009-08-28 13:20:34 +
+++ include/grub/term.h	2010-01-01 17:04:10 +
@@ -299,6 +299,14 @@ int EXPORT_FUNC(grub_getcursor) (void);
 void EXPORT_FUNC(grub_refresh) (void);
 void EXPORT_FUNC(grub_set_more) (int onoff);
 
+static inline void
+grub_print_spaces (int number_spaces)
+{
+  while (--number_spaces >= 0)
+  grub_putchar (' ');
+}
+
+
 /* For convenience.  */
 #define GRUB_TERM_ASCII_CHAR(c)	((c) & 0xff)
 

=== modified file 'normal/menu_text.c'
--- normal/menu_text.c	2009-12-27 21:32:52 +
+++ normal/menu_text.c	2010-01-01 15:48:11 +
@@ -45,14 +45,6 @@ grub_wait_after_message (void)
   grub_putchar ('\n');
 }
 
-static void
-print_spaces (int number_spaces)
-{
-  int i;
-  for (i = 0; i < number_spaces; i++)
-grub_putchar (' ');
-}
-
 void
 grub_print_ucs4 (const grub_uint32_t * str,
 const grub_uint32_t * last_position)
@@ -149,7 +141,7 @@ grub_print_message_indented (const char 
(grub_uint32_t *) last_position : next_new_line + line_len;
}
 
-  print_spaces (margin_left);
+  grub_print_spaces (margin_left);
   grub_print_ucs4 (current_position, next_new_line);
 
   next_new_line++;
@@ -405,7 +397,7 @@ print_timeout (int timeout, int offset)
  
   int posx;
   posx = grub_getxy() >> 8;
-  print_spaces (GRUB_TERM_WIDTH - posx - 1);
+  grub_print_spaces (GRUB_TERM_WIDTH - posx - 1);
 
   grub_gotoxy (GRUB_TERM_CURSOR_X, GRUB_TERM_FIRST_ENTRY_Y + offset);
   grub_refresh ();
@@ -495,7 +487,7 @@ run_menu (grub_menu_t menu, int nested, 
 	  if (timeout >= 0)
 	{
 	  grub_gotoxy (0, GRUB_TERM_HEIGHT - 3);
-  print_spaces (GRUB_TERM_WIDTH - 1);
+  grub_print_spaces (GRUB_TERM_WIDTH - 1);
 
 	  grub_env_un

Re: [PATCH] help command fix (II)

[Carles Pina i Estany]

Hi,

On Jan/01/2010, Carles Pina i Estany wrote:

> Find attached a new version, measuring the width of the strings. I have

After some comments from Vladimir on IRC, committed.

-- 
Carles Pina i Estany
http://pinux.info


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

revert in grub2

[Mihamina Rakotomandimby]

Manao ahoana, Hello, Bonjour,

How to reproduce the "revert" LILO feature with GRUB2?

I believe everyone here knows what it is, but a quick explanation for
those not knowing:
With LILO, I can setup a default image to boot on.
But, I also can setup the next entry to boot on, that may be different
to the default one.
After that first following boot, the default entry will be used.

What usage? 
It is usefull for me when testing a kernel on a remote
server. I set the default to a reliable boot image. I set the "revert"
to the testing one. I boot: it boots on the "revert" one.
If it ever doesnt boot, I initiate a hard reboot (electric cutoff) and
then it boots on the safe one.

I play with LILO just for that feature, I would like to switch to GRUB.

Misaotra, Thanks, Merci.

-- 
   Architecte Informatique chez Blueline/Gulfsat:
Administration Systeme, Recherche & Developpement
+261 34 29 155 34 / +261 33 11 207 36


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

Re: Year 2010

[Alfred M. Szmidt]

   Since it's not a lot of effort, I prefer to strive for correctness.  But
   thanks for the tip :-)

It wasn't a `tip', it is what is recommended by the FSF lawyers.  From
(maint)Copyright Notices:

| To update the list of year numbers, add each year in which you have
| made nontrivial changes to the package.  (Here we assume you're using
| a publicly accessible revision control server, so that every revision
| installed is also immediately and automatically published.)  When you
| add the new year, it is not required to keep track of which files have
| seen significant changes in the new year and which have not.  It is
| recommended and simpler to add the new year to all files in the
| package, and be done with it for the rest of the year.


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

Re: [PATCH] spaces (label, UUID)

[Isaac Dupree]

Carles Pina i Estany wrote:

+ grub_putchar (' ');
+ grub_printf_ (N_("- Label \"%s\""), label);
...

Avoiding to have trailing spaces helps the translators (they will do
less mistakes).


In this case it is a space at the beginning, not the end of the string 
-- does it still have this problem for translators? Or are you perhaps 
just trying to make code a bit more consistent re: beginning/end of strings?


-Isaac


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

Re: Roadmap for LUA support in GRUB

[Bruce O. Benson]

On Fri, Jan 1, 2010 at 6:42 AM, Robert Millan  wrote:

> On Mon, Dec 28, 2009 at 11:24:08PM -0500, Bruce O. Benson wrote:
> > As soon as I see a bootloader that uses Lua as its scripting/config
> engine,
> > I'm switching to it.
>
> We have LUA support for GRUB.  It's in grub-extras.
>
>
Exactly.  I didn't say 'support'.  I said 'uses'.

-- 
Bruce O. Benson, mailto:bben...@gmail.com | http://www.tux.org
Donating spare CPU to science: fold...@home Team Debian (#2019)
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel

Re: grub.info

[Bruce O. Benson]

On Fri, Jan 1, 2010 at 7:18 AM, Robert Millan  wrote:

> > This is the typical problem of contributing complaints vs. code.
>
> Bruce, there's absolutely no problem with Vladimir helping with review of
> code, documentation and proposals in general.  In fact, I find this very
> helpful.
>
> You too are welcome to provide your feedback and participate in the
> discussions.  If you do this, I will appreciate that you stay on topic
> and respectful to others.
>
> Thank you
>
>
This comment suggests that I have not been on topic or respectful of
others.  Is that correct?

-- 
Bruce O. Benson, mailto:bben...@gmail.com | http://www.tux.org
Donating spare CPU to science: fold...@home Team Debian (#2019)
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel