Re: Re: [patch] GRUB possible patches

2010-02-16 Thread gburanov
> First part applied
Thanks!

> Please don't put unrelated patches into single file. Fast look on the
> joliet.c of mkisofs suggests that it's not the case for its joliet. It
> seems there is more to it. Can you find some normative documents?

I searched a bit and found the following information:
Doc: http://www.transelectro.ru/technology/standard/joliet_spec.htm
In the documentation: 

"Simply put, SEPARATOR 1 and SEPARATOR 2 shall be expanded to 16-bits".

SEPARATOR 2 is ';' - which is used signify the version number of a file.

About mkisofs - it seems it's just their implementation of joilet, they can use 
or not use versioning
See this topic
http://www.mail-archive.com/cdwr...@other.debian.org/msg03199.html

> We use only C-style comments
Ok, I will create new patch soon

-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko





--
This message was sent on behalf of gbura...@gmail.com at openSubscriber.com
http://www.opensubscriber.com/message/grub-devel@gnu.org/13437133.html


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


Re: Re: What's the point of allocating the protected mode code on 0x100000 only (UEFI)

2010-02-16 Thread gburanov
> As I already said I have a working prototype for this in newreloc branch.

Thanks you, it works!
-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko




--
This message was sent on behalf of gbura...@gmail.com at openSubscriber.com
http://www.opensubscriber.com/message/grub-devel@gnu.org/13437030.html


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


[patch] scrolling bug

2010-02-16 Thread Michal Suchanek
Apparently it is the case.

Please test/apply.

Thanks

Michal
=== modified file 'video/fb/video_fb.c'
--- video/fb/video_fb.c	2010-02-03 00:24:07 +
+++ video/fb/video_fb.c	2010-02-16 23:44:09 +
@@ -985,6 +985,13 @@ grub_video_fb_scroll (grub_video_color_t
   linedelta = target.mode_info->pitch
 	- width * target.mode_info->bytes_per_pixel;
   linelen = width * target.mode_info->bytes_per_pixel;
+  if (dy > 0 || (dy == 0 && dx > 0))
+{
+  src_x += width - 1;
+  dst_x += width - 1;
+  src_y += height - 1;
+  dst_y += height - 1;
+}
 #define DO_SCROLL\
   /* Check vertical direction of the move.  */   \
   if (dy < 0 || (dy == 0 && dx < 0)) \
@@ -1006,11 +1013,9 @@ grub_video_fb_scroll (grub_video_color_t
 	{\
 	  /* 3b. Move data downwards.  */\
 	  dst = (void *) grub_video_fb_get_video_ptr (&target,	 \
-	 dst_x + width - 1,  \
-	 dst_y + height - 1);\
+		  dst_x, dst_y); \
 	  src = (void *) grub_video_fb_get_video_ptr (&target,	 \
-	 src_x + width - 1,  \
-	 src_y + height - 1);\
+		  src_x, src_y); \
 	  for (j = 0; j < height; j++)   \
 	{\
 	  for (i = 0; i < linelen; i++)  \

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


Re: scrolling bug

2010-02-16 Thread Michal Suchanek
Hello

is it by any chance possible that while the tests in grub_fb_scroll
check if the start is aligned the (width -1) which is added when
scrolling backwards is not?

This would explain why scrolling backwards is misaligned.

This would require to run non-32bpp mode but I am not sure how to tell
which mode I am running.

Thanks

Michal


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


Re: [RFC] Framebuffer rotation patch

2010-02-16 Thread Michal Suchanek
On 16 February 2010 22:14, richardvo...@gmail.com
 wrote:
> On Tue, Feb 16, 2010 at 3:05 PM, Michal Suchanek  wrote:
>> 2010/2/16 Vladimir 'φ-coder/phcoder' Serbinenko :
>>> Michal Suchanek wrote:
> With typeof macro this can be made type-neutral avoiding potential 
> mistakes.
> +static inline long
> +grub_min (long x, long y)
> +{
> +  if (x > y)
> +    return y;
> +  else
> +    return x;
> +}
> +
>

 I don't see how typeof would be used. As I understand the docs it can
 only set types relative to something what is already defined (and in
 some cases actually dereference/call it) and there is nothing defined
 at the point these functions are declared to copy the type from.

>>> #include 
>>> #define swap(a,b) {typeof (a) mytemp ## __LINE__; mytemp ## __LINE__ =
>>> b; b = a; a = mytemp ## __LINE__; }
>>>
>>
>> Unlike inlines this pollutes the local namespace with unexpected
>> identifiers.. Perhaps the temporary variable should be at least
>> prefixed with an underscore or something.
>
> The braces introduce a block and the variable goes out of scope, in
> fact there's no need for __LINE__ because of this.

It breaks things when you do have a mytemp already:

$ gcc -Wall testdef.c
$ ./a.out
2 1 3 4
$ cat testdef.c
#include 
#define swap(a,b) {typeof (a) mytemp; mytemp = b; b = a; a = mytemp; }

int main( int argc, char ** argv)
{
  int x=1, y=2, z=3, mytemp=4;
  swap(x,y);
  swap(z,mytemp);
  return printf("%i %i %i %i\n", x, y, z, mytemp);
}

Thanks

Michal


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


Re: [PATCH][UPDATED] support for xz compression format

2010-02-16 Thread Vladimir 'φ-coder/phcoder' Serbinenko
Szymon Janc wrote:
> Dnia wtorek 16 luty 2010 o 22:11:50 richardvo...@gmail.com napisał(a):
>
>   
>> Since gzip format allows decompression in pipeline mode, it's a
>> virtual certainty that nothing from the footer is required for
>> processing.
>>
>> And of course this is true of the xz format as well.  I quote from the
>> documentation:
>> 
>
> The only reason for checking footer is to get uncompressed data size to keep 
> grub_file_read() happy.
>
> Possible sollutions to avoid this:
> - add size field in stream header and break compatibility with upstream 
> xz/gz, 
> will require forking upstream compression tools or create special tool for 
> crafting upstream created files
>   
> - increase size while consuming blocks (possible with xz, don't know if with 
> gz), this leaves possibility to get grub_file_read() unhappy
>
> - try to guess uncompressed data size based on compressed size
>
>   
Solution number 4: make grub_file_read less exigent. If brainRAM serves
well file size is used only for offset checking and grub_file_size. In
first case offset checking can be effectivily disabled by setting size
to 0x. Main use of grub_file_size is to allocate a
buffer to load whole file at once. For these cases we may want to make a
grub_file_malloc_read which would arbitrate between allocation
strategies e.g. if fs->load_malloc is NULL use standard allocate/read,
otherwise call load_malloc. It is possible that some other code uses
grub_file_size (or file->size which is much uglier) in a slightly
different way but I think this solution should cover most if not all cases.
Specialised loaders may use guestimated compressed ratio, when it's
overflown (unlikely if estimation method is good), save overflow
separately by blocks and concatenate when done


-- 
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][UPDATED] support for xz compression format

2010-02-16 Thread Szymon Janc
Dnia wtorek 16 luty 2010 o 22:11:50 richardvo...@gmail.com napisał(a):

> Since gzip format allows decompression in pipeline mode, it's a
> virtual certainty that nothing from the footer is required for
> processing.
> 
> And of course this is true of the xz format as well.  I quote from the
> documentation:

The only reason for checking footer is to get uncompressed data size to keep 
grub_file_read() happy.

Possible sollutions to avoid this:
- add size field in stream header and break compatibility with upstream xz/gz, 
will require forking upstream compression tools or create special tool for 
crafting upstream created files
  
- increase size while consuming blocks (possible with xz, don't know if with 
gz), this leaves possibility to get grub_file_read() unhappy

- try to guess uncompressed data size based on compressed size

-- 
Szymon K. Janc
szy...@janc.net.pl // GG: 1383435


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


Re: [RFC] Framebuffer rotation patch

2010-02-16 Thread Michal Suchanek
On 16 February 2010 22:05, Michal Suchanek  wrote:
> 2010/2/16 Vladimir 'φ-coder/phcoder' Serbinenko :
>> Michal Suchanek wrote:
 With typeof macro this can be made type-neutral avoiding potential 
 mistakes.
 +static inline long
 +grub_min (long x, long y)
 +{
 +  if (x > y)
 +    return y;
 +  else
 +    return x;
 +}
 +

>>>
>>> I don't see how typeof would be used. As I understand the docs it can
>>> only set types relative to something what is already defined (and in
>>> some cases actually dereference/call it) and there is nothing defined
>>> at the point these functions are declared to copy the type from.
>>>
>> #include 
>> #define swap(a,b) {typeof (a) mytemp ## __LINE__; mytemp ## __LINE__ =
>> b; b = a; a = mytemp ## __LINE__; }
>>
>
> Unlike inlines this pollutes the local namespace with unexpected
> identifiers.. Perhaps the temporary variable should be at least
> prefixed with an underscore or something.
>
> But if that is ok then perhaps the existing functions in
> include/grub/misc.h should be converted.
>
> There is grub_max, grub_abs and grub_div_roundup already.

Hmm, we don't have stdio.h in grub.

Perhaps it's no that good idea after all.

Thanks

Michal


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


Re: [PATCH][UPDATED] support for xz compression format

2010-02-16 Thread richardvo...@gmail.com
On Tue, Feb 16, 2010 at 3:28 PM, Seth Goldberg  wrote:
> y
>
> Quoting richardvo...@gmail.com, who wrote the following on Tue, 16 Feb 2010:
>
>> On Tue, Feb 16, 2010 at 12:58 PM, Szymon Janc  wrote:
>>>
>>> Dnia wtorek 16 luty 2010 o 14:44:45 Vladimir 'φ-coder/phcoder' Serbinenko
>>> napisał(a):
>>>
 + #define VLI_MAX_DIGITS 9
 Are you sure it's 9? It gives only 63 and not 64 bits
>>>
>>> It is a limitation of xz format.
>>>
 +   if (! test_header(file) || ! test_footer(file))
 +   {
 Seeking to the end of file is very expensive on pxe. Can it be skipped?
>>>
>>> xz stores uncompressed data size at the end of file, without it file size
>>> can't
>>> be set.
>>>
>>> gzio has same issue, it reads last 4 bytes of file to determine
>>> uncompressed
>>> data size.
>>
>> Then maybe use of a more sensible format should be strongly
>> encouraged.  For sequentially accessed devices like tape, it may be
>> expensive to return to the header to add the size.  For flash memory,
>> it may require an overwrite (and flash memory has fast random access,
>> this is pretty much the only case to prefer a footer).  And formats
>> which support pipelining of course can't rewind to update a header.
>> But in general the archive will be read many times for each time it is
>> created, so it's best to put the size in the place which is best for
>> the decompressor.
>>
>> Since gzip format allows decompression in pipeline mode, it's a
>> virtual certainty that nothing from the footer is required for
>> processing.
>
>  I know I've said this before, but it bears repeating: Don't forget about
> network booting!  This is precisely the problem for which we came up with a
> different solution for loading large compressed archives via tftp when PXE
> booting.  The assumption, of course, is that you can size a buffer maximally
> (with some sane maximum)  then shrink it once the archive has been
> decompressed.  That's what we do and it's served us will with Legacy GRUB
> for years now.

In the case of .xz, there appear to be block headers, where the
uncompressed size is stored up front.  So as long as separate blocks
can go into independent memory allocations, there'd be no need to
oversize the buffer.

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


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


Re: [PATCH][UPDATED] support for xz compression format

2010-02-16 Thread Seth Goldberg

y

Quoting richardvo...@gmail.com, who wrote the following on Tue, 16 Feb 2010:


On Tue, Feb 16, 2010 at 12:58 PM, Szymon Janc  wrote:

Dnia wtorek 16 luty 2010 o 14:44:45 Vladimir 'φ-coder/phcoder' Serbinenko
napisał(a):


+ #define VLI_MAX_DIGITS 9
Are you sure it's 9? It gives only 63 and not 64 bits

It is a limitation of xz format.


+   if (! test_header(file) || ! test_footer(file))
+   {
Seeking to the end of file is very expensive on pxe. Can it be skipped?

xz stores uncompressed data size at the end of file, without it file size can't
be set.

gzio has same issue, it reads last 4 bytes of file to determine uncompressed
data size.


Then maybe use of a more sensible format should be strongly
encouraged.  For sequentially accessed devices like tape, it may be
expensive to return to the header to add the size.  For flash memory,
it may require an overwrite (and flash memory has fast random access,
this is pretty much the only case to prefer a footer).  And formats
which support pipelining of course can't rewind to update a header.
But in general the archive will be read many times for each time it is
created, so it's best to put the size in the place which is best for
the decompressor.

Since gzip format allows decompression in pipeline mode, it's a
virtual certainty that nothing from the footer is required for
processing.


  I know I've said this before, but it bears repeating: Don't forget about 
network booting!  This is precisely the problem for which we came up with a 
different solution for loading large compressed archives via tftp when PXE 
booting.  The assumption, of course, is that you can size a buffer maximally 
(with some sane maximum)  then shrink it once the archive has been 
decompressed.  That's what we do and it's served us will with Legacy GRUB for 
years now.


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


Re: [RFC] Framebuffer rotation patch

2010-02-16 Thread richardvo...@gmail.com
On Tue, Feb 16, 2010 at 3:05 PM, Michal Suchanek  wrote:
> 2010/2/16 Vladimir 'φ-coder/phcoder' Serbinenko :
>> Michal Suchanek wrote:
 With typeof macro this can be made type-neutral avoiding potential 
 mistakes.
 +static inline long
 +grub_min (long x, long y)
 +{
 +  if (x > y)
 +    return y;
 +  else
 +    return x;
 +}
 +

>>>
>>> I don't see how typeof would be used. As I understand the docs it can
>>> only set types relative to something what is already defined (and in
>>> some cases actually dereference/call it) and there is nothing defined
>>> at the point these functions are declared to copy the type from.
>>>
>> #include 
>> #define swap(a,b) {typeof (a) mytemp ## __LINE__; mytemp ## __LINE__ =
>> b; b = a; a = mytemp ## __LINE__; }
>>
>
> Unlike inlines this pollutes the local namespace with unexpected
> identifiers.. Perhaps the temporary variable should be at least
> prefixed with an underscore or something.

The braces introduce a block and the variable goes out of scope, in
fact there's no need for __LINE__ because of this.


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


Re: [PATCH][UPDATED] support for xz compression format

2010-02-16 Thread richardvo...@gmail.com
On Tue, Feb 16, 2010 at 12:58 PM, Szymon Janc  wrote:
> Dnia wtorek 16 luty 2010 o 14:44:45 Vladimir 'φ-coder/phcoder' Serbinenko
> napisał(a):
>
>> + #define VLI_MAX_DIGITS 9
>> Are you sure it's 9? It gives only 63 and not 64 bits
> It is a limitation of xz format.
>
>> +   if (! test_header(file) || ! test_footer(file))
>> +   {
>> Seeking to the end of file is very expensive on pxe. Can it be skipped?
> xz stores uncompressed data size at the end of file, without it file size 
> can't
> be set.
>
> gzio has same issue, it reads last 4 bytes of file to determine uncompressed
> data size.

Then maybe use of a more sensible format should be strongly
encouraged.  For sequentially accessed devices like tape, it may be
expensive to return to the header to add the size.  For flash memory,
it may require an overwrite (and flash memory has fast random access,
this is pretty much the only case to prefer a footer).  And formats
which support pipelining of course can't rewind to update a header.
But in general the archive will be read many times for each time it is
created, so it's best to put the size in the place which is best for
the decompressor.

Since gzip format allows decompression in pipeline mode, it's a
virtual certainty that nothing from the footer is required for
processing.

And of course this is true of the xz format as well.  I quote from the
documentation:

"Streamable: It is always possible to create and decompress .xz files
in a pipe; no seeking is required."
http://tukaani.org/xz/format.html


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


Re: [RFC] Framebuffer rotation patch

2010-02-16 Thread Michal Suchanek
2010/2/16 Vladimir 'φ-coder/phcoder' Serbinenko :
> Michal Suchanek wrote:
>>> With typeof macro this can be made type-neutral avoiding potential mistakes.
>>> +static inline long
>>> +grub_min (long x, long y)
>>> +{
>>> +  if (x > y)
>>> +    return y;
>>> +  else
>>> +    return x;
>>> +}
>>> +
>>>
>>
>> I don't see how typeof would be used. As I understand the docs it can
>> only set types relative to something what is already defined (and in
>> some cases actually dereference/call it) and there is nothing defined
>> at the point these functions are declared to copy the type from.
>>
> #include 
> #define swap(a,b) {typeof (a) mytemp ## __LINE__; mytemp ## __LINE__ =
> b; b = a; a = mytemp ## __LINE__; }
>

Unlike inlines this pollutes the local namespace with unexpected
identifiers.. Perhaps the temporary variable should be at least
prefixed with an underscore or something.

But if that is ok then perhaps the existing functions in
include/grub/misc.h should be converted.

There is grub_max, grub_abs and grub_div_roundup already.


Thanks

Michal


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


Re: [RFC] Framebuffer rotation patch, or why 'unsigned' fails us

2010-02-16 Thread Michal Suchanek
2010/2/16 Vladimir 'φ-coder/phcoder' Serbinenko :
> Michal Suchanek wrote:
>> This also enhances the video interface so that drawing at negative
>> position is representable in the arguments. Iit is then possible to
>> blit a bitmap without first clipping the topleft part by just blitting
>> at negative position which should make some operations easier to carry
>> out.
>>
> It's a good reason then. Just be sure that every code reacts sanely to
> negative integers either by sanitising or correctly handling

Actually it was possible to blit a bitmap at negative position but it
was not possile to draw a rectangle at the same position which was
somewhat odd.

Thanks

Michal


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


Re: [PATCH] transparent file reader

2010-02-16 Thread Szymon Janc
Dnia wtorek 16 luty 2010 o 20:38:28 Seth Goldberg napisał(a):
>   How do consumers of this interface determine the underlying cause of the
> problem?  Sometimes, more layering creates more problems than it solves.
> Think about how you determine the message to display when getting a return
> value from a function.

Right, should check grub_errno when filter returns null file handler.
I'll add that in next version of patch.

-- 
Szymon K. Janc
szy...@janc.net.pl // GG: 1383435


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


Re: [PATCH] transparent file reader

2010-02-16 Thread Seth Goldberg


 How do consumers of this interface determine the underlying cause of the 
problem?  Sometimes, more layering creates more problems than it solves. 
Think about how you determine the message to display when getting a return 
value from a function.


 --S

Quoting Szymon Janc, who wrote the following on Tue, 16 Feb 2010:


Hello,

Attached patch makes file reader transparent. It should make adding new filters
to file reader easier.

- gzio.h is gone
- gzio is no more transparent, transparency is handled in grub_file_open()
- renamed GRUB_ERR_BAD_GZIP_DATA to GRUB_ERR_BAD_IOFILTER_DATA, it will be
used by other ios like xzio


What do You think about it?


--
Szymon K. Janc
szy...@janc.net.pl // GG: 1383435




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


Re: [PATCH] transparent file reader

2010-02-16 Thread Szymon Janc
Dnia wtorek 16 luty 2010 o 19:16:19 Vladimir 'φ-coder/phcoder' Serbinenko 
napisał(a):

> I like the patch however few comments:
> 1) How are filters ordered?

Filters are tested in order that last registered filter is tested first. First 
match stop further tests.

> 2) How would I selectively disable a filter. E.g. for hexdump or when
> payload expects compressed data?

Currently one would have to unregister filters before calling grub_file_open()
and reregister them afterwards. However this can mix up filters order. Not 
a best option.

I was thinking about something like:
grub_file_open("name", NO_XFILTER|NO_YFILTER);
grub_file_open("name",NO_ALL);
to allow selectable filters or fopen() style maybe.

And a priority based filter registration if there is a need for that.


-- 
Szymon K. Janc
szy...@janc.net.pl // GG: 1383435


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


Re: [PATCH][UPDATED] support for xz compression format

2010-02-16 Thread Szymon Janc
Dnia wtorek 16 luty 2010 o 14:44:45 Vladimir 'φ-coder/phcoder' Serbinenko 
napisał(a):

> + #define VLI_MAX_DIGITS 9
> Are you sure it's 9? It gives only 63 and not 64 bits
It is a limitation of xz format.

> +   if (! test_header(file) || ! test_footer(file))
> +   {
> Seeking to the end of file is very expensive on pxe. Can it be skipped?
xz stores uncompressed data size at the end of file, without it file size can't 
be set.

gzio has same issue, it reads last 4 bytes of file to determine uncompressed 
data size.

> grub_xzio_read seems to ignore file->offset. Have you tested seeking?
Yes, seeking is not implemented yet. It's on my TODO list.


-- 
Szymon K. Janc
szy...@janc.net.pl // GG: 1383435


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


[Off-topic] C++ enums

2010-02-16 Thread Isaac Dupree

On 02/16/10 13:15, richardvo...@gmail.com wrote:

On Tue, Feb 16, 2010 at 12:03 PM, Isaac Dupree
  wrote:

On 02/16/10 10:52, Michal Suchanek wrote:


enum allows it just fine


Not here:

typedef enum t1 { BTI1 = 1,


typo, should be "BIT1". then it works. (In C.  Also remember not to get
confused by the fact that it doesn't work in C++, for type-related reasons


Says who?


OK, I guess I'll get into it, since you asked...


Comeau is perfectly happy with this code, in strict C++ mode:

enum flags
{
   BIT1 = 1,
   BIT2 = 2,
   BIT12 = BIT1 | BIT2
};


In C and C++, enums are their own type distinct from int.  In C, there 
exist implicit conversions both from and to all enum types and int.  In 
C++, there only exist implicit conversions from enum types to int. 
Bitwise and arithmetic operators only operate on int, not enum types 
(unless a C++ operator overload is declared); these operators appear to 
work with enums because of all the implicit conversion.


Your example works even in C++ because the right-hand side of an "=" in 
an enum-declaration is of type int, rather than the enum type (as you 
can see from the fact that you can put "1" there as well).  Now BIT1, 
BIT2, and BIT12 are all (in C++) values (rvalues) of type "flags" (note 
this does not apply to the left-hand sides in the enum declaration that 
define their constant values, but it does apply to those right-hand sides).


Now try and combine your with this main function:

int main(int argc, char** argv) {
flags foo1 = BIT1; // fine
flags foo12 = BIT12; // fine
int   bar = BIT1 | BIT2; // fine
flags baz = BIT1 | BIT2; // error
return 0 ;
}

(If you want to see it compile in C, add "typedef enum flags flags;" 
before main().)


See http://www.parashift.com/c++-faq-lite/newbie.html#faq-29.19 for some 
discussion by a C++ expert.  And then return to GRUB2 coding where none 
of this matters.  The only weird thing about enums in C is that they're 
not guaranteed by the standard to be isomorphic to type "int"; each enum 
might correspond to a different-size, (possibly even 
different-signedness), integral type, if this is possible given the 
range of values in the particular "enum{...};" declaration. (This weird 
thing also affects C++ but I omitted it in the above explanation of enum 
versus integral types.)


-Isaac


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


Re: [PATCH] transparent file reader

2010-02-16 Thread Vladimir 'φ-coder/phcoder' Serbinenko
Szymon Janc wrote:
> Hello,
>
> Attached patch makes file reader transparent. It should make adding new 
> filters 
> to file reader easier.
>
> - gzio.h is gone
> - gzio is no more transparent, transparency is handled in grub_file_open()
> - renamed GRUB_ERR_BAD_GZIP_DATA to GRUB_ERR_BAD_IOFILTER_DATA, it will be
>  used by other ios like xzio
>
>   
I like the patch however few comments:
1) How are filters ordered?
2) How would I selectively disable a filter. E.g. for hexdump or when
payload expects compressed data?

+/* Registered filters list.  */
+static grub_io_filter_t grub_io_filter_list = NULL;
+
+void grub_io_register(grub_io_filter_t filter)
+{
+   filter->next = grub_io_filter_list;
+   grub_io_filter_list = filter;
+}
+

list.h can be used for this.

> What do You think about it?
>
>
>   
> 
>
> ___
> 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: [RFC] Framebuffer rotation patch

2010-02-16 Thread richardvo...@gmail.com
On Tue, Feb 16, 2010 at 12:03 PM, Isaac Dupree
 wrote:
> On 02/16/10 10:52, Michal Suchanek wrote:
>>>
>>> enum allows it just fine
>>
>> Not here:
>>
>> typedef enum t1 { BTI1 = 1,
>
> typo, should be "BIT1". then it works. (In C.  Also remember not to get
> confused by the fact that it doesn't work in C++, for type-related reasons

Says who?

Comeau is perfectly happy with this code, in strict C++ mode:

enum flags
{
  BIT1 = 1,
  BIT2 = 2,
  BIT12 = BIT1 | BIT2
};

int main(int argc, char** argv) { return 0 ; }


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


[PATCH] transparent file reader

2010-02-16 Thread Szymon Janc
Hello,

Attached patch makes file reader transparent. It should make adding new filters 
to file reader easier.

- gzio.h is gone
- gzio is no more transparent, transparency is handled in grub_file_open()
- renamed GRUB_ERR_BAD_GZIP_DATA to GRUB_ERR_BAD_IOFILTER_DATA, it will be
 used by other ios like xzio


What do You think about it?


-- 
Szymon K. Janc
szy...@janc.net.pl // GG: 1383435
diff -urBN orig//experimental/commands/acpi.c transparentio//experimental/commands/acpi.c
--- orig//experimental/commands/acpi.c	2010-02-16 01:43:03.711945824 +0100
+++ transparentio//experimental/commands/acpi.c	2010-02-16 18:29:04.037133199 +0100
@@ -23,7 +23,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -627,7 +626,7 @@
   grub_size_t size;
   char *buf;
 
-  file = grub_gzfile_open (args[i], 1);
+  file = grub_file_open (args[i]);
   if (! file)
 	{
 	  free_tables ();
diff -urBN orig//experimental/commands/cat.c transparentio//experimental/commands/cat.c
--- orig//experimental/commands/cat.c	2010-02-16 01:43:03.755946959 +0100
+++ transparentio//experimental/commands/cat.c	2010-02-16 18:29:04.037133199 +0100
@@ -22,7 +22,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
@@ -39,7 +38,7 @@
   if (argc != 1)
 return grub_error (GRUB_ERR_BAD_ARGUMENT, "file name required");
 
-  file = grub_gzfile_open (args[0], 1);
+  file = grub_file_open (args[0]);
   if (! file)
 return 0;
 
diff -urBN orig//experimental/commands/cmp.c transparentio//experimental/commands/cmp.c
--- orig//experimental/commands/cmp.c	2010-02-16 01:43:03.783945497 +0100
+++ transparentio//experimental/commands/cmp.c	2010-02-16 18:29:04.037133199 +0100
@@ -21,7 +21,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
@@ -44,8 +43,8 @@
   grub_printf ("Compare file `%s' with `%s':\n", args[0],
 	   args[1]);
 
-  file1 = grub_gzfile_open (args[0], 1);
-  file2 = grub_gzfile_open (args[1], 1);
+  file1 = grub_file_open (args[0]);
+  file2 = grub_file_open (args[1]);
   if (! file1 || ! file2)
 goto cleanup;
 
diff -urBN orig//experimental/commands/hexdump.c transparentio//experimental/commands/hexdump.c
--- orig//experimental/commands/hexdump.c	2010-02-16 01:43:03.900945869 +0100
+++ transparentio//experimental/commands/hexdump.c	2010-02-16 18:29:04.037133199 +0100
@@ -21,7 +21,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -89,7 +88,7 @@
 {
   grub_file_t file;
 
-  file = grub_gzfile_open (args[0], 1);
+  file = grub_file_open (args[0]);
   if (! file)
 	return 0;
 
diff -urBN orig//experimental/gettext/gettext.c transparentio//experimental/gettext/gettext.c
--- orig//experimental/gettext/gettext.c	2010-02-16 01:43:03.859945957 +0100
+++ transparentio//experimental/gettext/gettext.c	2010-02-16 18:29:04.100132573 +0100
@@ -26,7 +26,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 
 /*
@@ -229,7 +228,7 @@
   /* Using fd_mo and not another variable because
  it's needed for grub_gettext_get_info.  */
 
-  fd_mo = grub_gzfile_open (filename, 1);
+  fd_mo = grub_file_open (filename);
   grub_errno = GRUB_ERR_NONE;
 
   if (!fd_mo)
diff -urBN orig//experimental/include/grub/err.h transparentio//experimental/include/grub/err.h
--- orig//experimental/include/grub/err.h	2010-02-16 01:43:03.835943739 +0100
+++ transparentio//experimental/include/grub/err.h	2010-02-16 18:29:04.116149419 +0100
@@ -50,7 +50,7 @@
 GRUB_ERR_BAD_FONT,
 GRUB_ERR_NOT_IMPLEMENTED_YET,
 GRUB_ERR_SYMLINK_LOOP,
-GRUB_ERR_BAD_GZIP_DATA,
+GRUB_ERR_BAD_IOFILTER_DATA,
 GRUB_ERR_MENU,
 GRUB_ERR_TIMEOUT,
 GRUB_ERR_IO,
diff -urBN orig//experimental/include/grub/file.h transparentio//experimental/include/grub/file.h
--- orig//experimental/include/grub/file.h	2010-02-16 01:43:03.843945875 +0100
+++ transparentio//experimental/include/grub/file.h	2010-02-16 18:29:04.116149419 +0100
@@ -48,6 +48,20 @@
 };
 typedef struct grub_file *grub_file_t;
 
+/* The io filter structure.  */
+struct grub_io_filter
+{
+  grub_file_t (*grub_io_open)(grub_file_t file);
+  struct grub_io_filter *next;
+};
+typedef struct grub_io_filter *grub_io_filter_t;
+
+/* Register io filter.  */
+void EXPORT_FUNC(grub_io_register) (grub_io_filter_t filter);
+
+/* Unregister io filter.  */
+void EXPORT_FUNC(grub_io_unregister) (grub_io_filter_t filter);
+
 /* Get a device name from NAME.  */
 char *EXPORT_FUNC(grub_file_get_device_name) (const char *name);
 
diff -urBN orig//experimental/include/grub/gzio.h transparentio//experimental/include/grub/gzio.h
--- orig//experimental/include/grub/gzio.h	2010-02-16 01:43:03.896944801 +0100
+++ transparentio//experimental/include/grub/gzio.h	1970-01-01 01:00:00.0 +0100
@@ -1,28 +0,0 @@
-/* gzio.h - prototypes for gzio */
-/*
- *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2005,2007  Free Software Foundation, Inc.
- *
- *  GRUB is free software: you ca

Re: [RFC] Framebuffer rotation patch

2010-02-16 Thread Isaac Dupree

On 02/16/10 10:52, Michal Suchanek wrote:

enum allows it just fine


Not here:

typedef enum t1 { BTI1 = 1,


typo, should be "BIT1". then it works. (In C.  Also remember not to get 
confused by the fact that it doesn't work in C++, for type-related 
reasons that we don't need to get into here because GRUB is written in C.)



   BIT2 = 2,
   BIT12 = BIT1 + BIT2} t;

int main(int argc, char** argv) { return 0 ; }

testenum.c:3: error: ‘BIT1’ undeclared here (not in a function)


-Isaac


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


Re: [PATCH] delete and backspace; unimplemented keys

2010-02-16 Thread Manoel Rebelo Abraches
Hi.
As talked with phcoder in #grub on freenode, there is a cleaner version
of the first patch, taking in consideration the recent changes in
ofconsole.
There is also a little fix to get the "~" character when the "DEL" key
is pressed. 

On Thu, 2010-02-11 at 16:36 -0200, Manoel Rebelo Abraches wrote:
> Hello.
> We have made those simple patches to fix some problems with
> unimplemented keys in ieee1275 terminals.
> The first patch (ofconsole_keys) implements the "backspace" and "delete"
> keys.
> The second one (ofconsole_unimplemented_keys) will clean the buffer of
> unknown keys and prevent garbage to show up in the screen. This patch
> was tested in both P5 and P6 machines.
> Please review and let me know your opinion about it.
> Thank you.
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel


-- 
Best Regards,

Manoel Rebelo Abranches
Software engineer
IBM - Linux Technology Center - Brazil
=== modified file 'ChangeLog'
--- ChangeLog	2010-02-14 17:36:26 +
+++ ChangeLog	2010-02-16 16:41:50 +
@@ -130,6 +130,11 @@
 
 	* util/grub-mkrawimage.c (generate_image): Add forgotten ALIGN_UP.
 
+2010-02-16  Manoel Rebelo Abranches 
+
+	* term/ieee1275/ofconsole.c (grub_ofconsole_readkey): Add some
+	unimplemented keys.
+
 2010-02-10  Vladimir Serbinenko  
 
 	Pass SIMPLE framebuffer size in bytes and not 64K blocks.

=== modified file 'term/ieee1275/ofconsole.c'
--- term/ieee1275/ofconsole.c	2010-02-14 13:52:10 +
+++ term/ieee1275/ofconsole.c	2010-02-16 19:15:34 +
@@ -202,9 +202,16 @@
 
   grub_ieee1275_read (stdin_ihandle, &c, 1, &actual);
 
-  if (actual > 0 && c == '\e')
+  if (actual > 0)
+switch(c)
 {
   grub_uint64_t start;
+  case 0x7f:
+/* Backspace: Ctrl-h.  */
+c=8; 
+break;
+  case '\e':
+
   grub_ieee1275_read (stdin_ihandle, &c, 1, &actual);
 
   /* On 9600 we have to wait up to 12 milliseconds.  */
@@ -247,7 +254,25 @@
 	  /* Left: Ctrl-b.  */
 	  c = GRUB_TERM_LEFT;
 	  break;
+  	  case '3':
+
+grub_ieee1275_read (stdin_ihandle, &c, 1, &actual);
+/* On 9600 we have to wait up to 12 milliseconds.  */
+start = grub_get_time_ms ();
+while (actual <= 0 && grub_get_time_ms () - start < 12)
+  grub_ieee1275_read (stdin_ihandle, &c, 1, &actual);
+
+if (actual <= 0)
+  	  return 0;
+  	
+/* Delete: Ctrl-d.  */
+if (c == '~')
+  	  c = 4;
+else
+  return 0;
+  	break;
 	}
+  break;
 }
 
   *key = c;

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


Re: [RFC] Framebuffer rotation patch

2010-02-16 Thread Vladimir 'φ-coder/phcoder' Serbinenko
Michal Suchanek wrote:
>> With typeof macro this can be made type-neutral avoiding potential mistakes.
>> +static inline long
>> +grub_min (long x, long y)
>> +{
>> +  if (x > y)
>> +return y;
>> +  else
>> +return x;
>> +}
>> +
>> 
>
> I don't see how typeof would be used. As I understand the docs it can
> only set types relative to something what is already defined (and in
> some cases actually dereference/call it) and there is nothing defined
> at the point these functions are declared to copy the type from.
>   
#include 
#define swap(a,b) {typeof (a) mytemp ## __LINE__; mytemp ## __LINE__ =
b; b = a; a = mytemp ## __LINE__; }

int
main ()
{
  int x = 1, y = 2;
  swap (x,y);
  printf ("%d, %d\n", x, y);
}


-- 
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: [RFC] Framebuffer rotation patch

2010-02-16 Thread Michal Suchanek
2010/2/11 Vladimir 'φ-coder/phcoder' Serbinenko :
> Michal Suchanek wrote:
>> Hello
>>
>> Sending a preliminary framebuffer rotation patch.
>>
>> You can use videotest to see 4 tiles rotated from the same bitmap data.
>>
>>
> +char leaf_data[] = { 0x00, 0x0f, 0xe0, 0x00,
> +                     0x00, 0x7f, 0xfc, 0x00,
> +                     0x01, 0xff, 0xff, 0x00,
> +                     0x03, 0xff, 0xff, 0x80,
> This is a blob. Could it be generated automatically at build time?
> +
> +  sans = grub_font_get ("Helvetica Bold 14");
> Please use free font rather than Helvetica
> Could you split addition of videotests from the rest of the patch?
> -    unsigned int x;
> -    unsigned int y;
> -    unsigned int width;
> -    unsigned int height;
> +    int x;
> +    int y;
> +    int width;
> +    int height;
> Why do you need negative values?
> +/* Supported operations are simple and easy to understand.
> + * MIRROR  |  swap image across (around) the vertical axis
> + * FLIP    -  swap image across the horizontal axis - upside down
> + * SWAP    /  swap image across the x=y axis - swap the x and y coordinates
> It's just a D_8 group. Could you add a comment to functions what they do
> in group theoretical sense? It would make the code easier to follow and
> compute transformations for mathematicians. Perhaps another
> representation of D_8 would result in more efficient code?
> +static inline void grub_swap_int(int *a, int *b) { int tmp = *a; *a =
> *b; *b = tmp; }
> +static inline void grub_swap_unsigned(unsigned *a, unsigned *b) {
> unsigned tmp = *a; *a = *b; *b = tmp; }
> +
> With typeof macro this can be made type-neutral avoiding potential mistakes.
> +static inline long
> +grub_min (long x, long y)
> +{
> +  if (x > y)
> +    return y;
> +  else
> +    return x;
> +}
> +

I don't see how typeof would be used. As I understand the docs it can
only set types relative to something what is already defined (and in
some cases actually dereference/call it) and there is nothing defined
at the point these functions are declared to copy the type from.

Still the grub_min being long is somewhat suspicious. iirc I just
reused an existing function or copied grub_max.

If declaring it as long can cause problems then its utility as a
general purpose function is dubious.

Thanks

Michal


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


Re: [RFC] Framebuffer rotation patch

2010-02-16 Thread Michal Suchanek
2010/2/16 Vladimir 'φ-coder/phcoder' Serbinenko :
> Michal Suchanek wrote:
>> 2010/2/11 Vladimir 'φ-coder/phcoder' Serbinenko :
>>
>>> Michal Suchanek wrote:

>> This requires that both u and w be in the chosen set of generators
>> because otherwise use of v or s twice is required to get one from the
>> other. Using (u or w) and v as the two basic operations additionally
>> requires composing generators in non-fixed order to get all the 8
>> possible combinations.
>>
>> The other reason for having 3 generators is clear and efficient
>> reperesentation of the 8 possible transformations. They are
>> represented as bits in the bitmap where each bit specifies if the
>> particular basic transform is included or not but they are always
>> applied in fixed order and at most once.
>>
>> Compare this to your representation of s^k,s^k*t where k is 0..3.
>> Depending on representation the composition and inversion rules might
>> still be somewhat non-trivial in actual code, using two reflections
>> which require multiple applications of the two generators in
>> particular order would lead to even more "interesting" code.
>>
>>
> You don't have to use same representation through whole code

I don't see any benefit in changing it and it would make the code
harder  to understand.



>>> enum my_bitfield
>>> {
>>>  MY_BITFIELD_NONE=0,
>>>  MY_BITFIELD_BIT0 = 1 << 0,
>>>  MY_BITFIELD_BIT1 = 1 << 1,
>>>  MY_BITFIELD_BIT2 = 1 << 2
>>> };
>>>
>>
>> The whole point of a bitfield is to have values like
>> MY_BITFIELD_1_AND_2 = MY_BITFIELD_BIT1 | MY_BITFIELD_BIT2 which enum
>> does not allow.
>>
> enum allows it just fine

Not here:

typedef enum t1 { BTI1 = 1,
  BIT2 = 2,
  BIT12 = BIT1 + BIT2} t;

int main(int argc, char** argv) { return 0 ; }

testenum.c:3: error: ‘BIT1’ undeclared here (not in a function)



Thanks

Michal


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


Re: [PATCH][UPDATED] support for xz compression format

2010-02-16 Thread Vladimir 'φ-coder/phcoder' Serbinenko
Szymon Janc wrote:
> On Tue, 16 Feb 2010 14:12:04 +0100  Vladimir 'φ-coder/phcoder' Serbinenko 
>  wrote
>
>   
>> First of all: could you use unified diff? (-u option)
>> 
>
> Sure, will use that in future.
>
>   
>> grub2-1.98~experimental.20100120/conf/xzembed.rmk
>> I don't see a need for either separate .rmk or separate module
>>
>> + static grub_uint8_t inbuf[XZBUFSIZ];
>> + static grub_uint8_t outbuf[XZBUFSIZ];
>> Avoid static variables. It will fail if user e.g. mounts xz file as a
>> loopback then opens xz'ed file on this loopback. Just put buffers in
>> grub_xzio
>> +   if (! file)
>> +   {
>> +   grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
>> + return 0;
>> grub_error is already issued by grub_malloc, no need to put the same
>> code here.
>> 
>
> I think You have reviewed old patch, not the updated one :-)
>
>   
Right. Many comments still apply though. Reviewing new one (in addition
to old comments):
+ #define VLI_MAX_DIGITS 9
Are you sure it's 9? It gives only 63 and not 64 bits
+   if (! test_header(file) || ! test_footer(file))
+   {
Seeking to the end of file is very expensive on pxe. Can it be skipped?
grub_xzio_read seems to ignore file->offset. Have you tested seeking?

> 
>
> ___
> 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: avoid possible overflow?

2010-02-16 Thread richardvo...@gmail.com
2010/2/16 Vladimir 'φ-coder/phcoder' Serbinenko :
> Carles Pina i Estany wrote:
>> Hello,
>>
>> The common pattern when doing a search by bisection is something like:
>> +      current = min + (max - min) / 2;
>>
>> Instead of the first natural idea:
>> -      current = (max + min) / 2;
>>
>> To avoid overflows.
>>
>> In gettext/gettext.c it's used in the "incorrect" way. It's not a big
>> problem since would happen only with .mo files with lot of strings, like
>> number that int represents in that architecture divided by 2 (aprox
>> aprox.).
>>
>> See the attached file for a patch if we want to patch.
>>
> You forgot to make the same change before the loop. But actually it
> doesn't matter because overflow in this statement:
>  internal_position = offsettranslation + position * 8;
> is reached well before the overflow in bisecting. Anyway this
> mattersonly for >4GiB files and unless we put videos in .po I don't see
> how this limit would be reached. So I prefer readability and would
> reject this patch.

There's a very readable way to express the "better" code that's immune
to the overflow:

int const range = max - min; /* or unsigned int, if min and max are
declared that way */
current = min + range / 2;


>> Else I would at least add a comment that we simplified because we
>> consider that will not happen.
>>
>> Thanks,
>>
>>
>> 
>>
>> ___
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> http://lists.gnu.org/mailman/listinfo/grub-devel
>
>
> --
> Regards
> Vladimir 'φ-coder/phcoder' Serbinenko
>
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>
>


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


Re: [PATCH][UPDATED] support for xz compression format

2010-02-16 Thread Szymon Janc
On Tue, 16 Feb 2010 14:12:04 +0100  Vladimir 'φ-coder/phcoder' Serbinenko 
 wrote

> First of all: could you use unified diff? (-u option)

Sure, will use that in future.

> grub2-1.98~experimental.20100120/conf/xzembed.rmk
> I don't see a need for either separate .rmk or separate module
> 
> + static grub_uint8_t inbuf[XZBUFSIZ];
> + static grub_uint8_t outbuf[XZBUFSIZ];
> Avoid static variables. It will fail if user e.g. mounts xz file as a
> loopback then opens xz'ed file on this loopback. Just put buffers in
> grub_xzio
> +   if (! file)
> +   {
> +   grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
> + return 0;
> grub_error is already issued by grub_malloc, no need to put the same
> code here.

I think You have reviewed old patch, not the updated one :-)

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


Re: [PATCH][UPDATED] support for xz compression format

2010-02-16 Thread Vladimir 'φ-coder/phcoder' Serbinenko
Szymon Janc wrote:
> Hello,
>
> New version of xz compression patch.
> Changes since then:
> - it works now :-)
> - decoder dictionary can be enlarge up to DICT_BIT_SIZE defined in xz_lzma2.h
>  (currently set to 30 = 128MiB)
> - xz uses crc32 from libgcrypt-grub, internal crc implementation removed
> - removed linux kernel related code from xz
> - get rid of some not needed memcpy() calls
> - xzio.mod stuff made according to Vladimir's comments
> - simplified wraper (only 1 file - xz_wrap.h)
> - number of other improvments and tons of bugfixes
> - files licence changed from public domain to gpl3+
>
> This patch also changes gnulib-wrap.h true/false definitions to make them 
> truly
> constants.
>
> TODO and other questions:
> - lack of file seek support, if files is compressed with small block size, it 
> is 
>   possible to implement pseudo-random access
> - what is optimal i/o buffer size? like BUFSIZ macro in glibc
> - default dictionary size should be chosen for files compression
> - still need to do performance tests
> - introduce some common layer for xzio/gzio etc ?
>
> Comments are welcome
>
>   
First of all: could you use unified diff? (-u option)
  # Misc.
! pkglib_MODULES += gzio.mod bufio.mod elf.mod xzio.mod
Please put pkglib_MODULES += xzio.mod right before other variables

grub2-1.98~experimental.20100120/conf/xzembed.rmk
I don't see a need for either separate .rmk or separate module

+ static grub_uint8_t inbuf[XZBUFSIZ];
+ static grub_uint8_t outbuf[XZBUFSIZ];
Avoid static variables. It will fail if user e.g. mounts xz file as a
loopback then opens xz'ed file on this loopback. Just put buffers in
grub_xzio
+   if (! file)
+   {
+   grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
+ return 0;
grub_error is already issued by grub_malloc, no need to put the same
code here.
+   grub_memset (xzio, 0, sizeof (*xzio));
Use grub_zalloc instead of malloc + memset
+   xzio->buf.out_size = len >= XZBUFSIZ ? XZBUFSIZ : len;
I suggest adding parenthesis around condition
+ /* feed input */
Please capitalise first letter and terminate the comments with a full
stop and 2 spaces.
+ readret = grub_file_read(xzio->file,inbuf,XZBUFSIZ);
Missing spaces before opening bracket and after commas. I recommend
running indent on new files.
> 
>
> ___
> 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: beep support?

2010-02-16 Thread Vladimir 'φ-coder/phcoder' Serbinenko
Samuel Thibault wrote:
> Samuel Thibault, le Fri 05 Feb 2010 19:11:18 +0100, a écrit :
>   
>> Vladimir 'φ-coder/phcoder' Serbinenko, le Fri 05 Feb 2010 17:52:02 +0100, a 
>> écrit :
>> 
>>> Could you detail at which events beep should be produced and propose a
>>> patch to add appropriate hooks?
>>>   
>> A beep just when the menu is first drawn is already a good thing and is
>> trivial to add by just calling play in the main menu.
>> 
>
> Here is a proposed patch.  Is the GRUB_INIT_TUNE variable name Ok?
>
>   
This is small clean patch and it improves accessibility. Please go ahead.
> Samuel
>
> 2010-02-14  Samuel Thibault  
>
>   * util/grub-mkconfig.in: Export GRUB_INIT_TUNE.
>   * util/grub.d/00_header.in: Handle GRUB_INIT_TUNE.
>
> --- util/grub-mkconfig.in 2010-02-10 18:53:13 +
> +++ util/grub-mkconfig.in 2010-02-14 18:05:11 +
> @@ -222,7 +222,8 @@ export GRUB_DEFAULT \
>GRUB_GFXMODE \
>GRUB_THEME \
>GRUB_GFXPAYLOAD_LINUX \
> -  GRUB_DISABLE_OS_PROBER
> +  GRUB_DISABLE_OS_PROBER \
> +  GRUB_INIT_TUNE
>  
>  if test "x${grub_cfg}" != "x"; then
>rm -f ${grub_cfg}.new
>
> === modified file 'util/grub.d/00_header.in'
> --- util/grub.d/00_header.in  2010-02-03 00:24:07 +
> +++ util/grub.d/00_header.in  2010-02-14 17:55:04 +
> @@ -158,3 +158,11 @@ else
>  set timeout=${GRUB_TIMEOUT}
>  EOF
>  fi
> +
> +# Play an initial tune
> +if [ "x${GRUB_INIT_TUNE}" != "x" ] ; then
> +  cat << EOF
> +insmod play
> +play ${GRUB_INIT_TUNE}
> +EOF
> +fi
>
>
>
> ___
> 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: avoid possible overflow?

2010-02-16 Thread Vladimir 'φ-coder/phcoder' Serbinenko
Carles Pina i Estany wrote:
> Hello,
>
> The common pattern when doing a search by bisection is something like:
> +  current = min + (max - min) / 2;
>
> Instead of the first natural idea:
> -  current = (max + min) / 2;
>
> To avoid overflows.
>
> In gettext/gettext.c it's used in the "incorrect" way. It's not a big
> problem since would happen only with .mo files with lot of strings, like
> number that int represents in that architecture divided by 2 (aprox
> aprox.).
>
> See the attached file for a patch if we want to patch.
>   
You forgot to make the same change before the loop. But actually it
doesn't matter because overflow in this statement:
  internal_position = offsettranslation + position * 8;
is reached well before the overflow in bisecting. Anyway this
mattersonly for >4GiB files and unless we put videos in .po I don't see
how this limit would be reached. So I prefer readability and would
reject this patch.
> Else I would at least add a comment that we simplified because we
> consider that will not happen.
>
> Thanks,
>
>   
> 
>
> ___
> 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: [RFC] Framebuffer rotation patch, or why 'unsigned' fails us

2010-02-16 Thread Vladimir 'φ-coder/phcoder' Serbinenko
Michal Suchanek wrote:
> This also enhances the video interface so that drawing at negative
> position is representable in the arguments. Iit is then possible to
> blit a bitmap without first clipping the topleft part by just blitting
> at negative position which should make some operations easier to carry
> out.
>   
It's a good reason then. Just be sure that every code reacts sanely to
negative integers either by sanitising or correctly handling
>
> ___
> 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: [RFC] Framebuffer rotation patch

2010-02-16 Thread Vladimir 'φ-coder/phcoder' Serbinenko
Michal Suchanek wrote:
> 2010/2/11 Vladimir 'φ-coder/phcoder' Serbinenko :
>   
>> Michal Suchanek wrote:
>> 
>>> 2010/2/11 Vladimir 'φ-coder/phcoder' Serbinenko :
>>>
>>>   
 Michal Suchanek wrote:

 
> Hello
>
> Sending a preliminary framebuffer rotation patch.
>
> You can use videotest to see 4 tiles rotated from the same bitmap data.
>
>
>
>   
 +char leaf_data[] = { 0x00, 0x0f, 0xe0, 0x00,
 + 0x00, 0x7f, 0xfc, 0x00,
 + 0x01, 0xff, 0xff, 0x00,
 + 0x03, 0xff, 0xff, 0x80,
 This is a blob. Could it be generated automatically at build time?

 
>>> How less of a blob it would be if it was included as a bitmap?
>>>
>>>
>>>   
>> It's not easily editable in current form.
>> 
>>> This is just a shape used for the video tests.
>>>
>>> There are some X tools for working with bitmaps/pixmaps which could
>>> generate this from a viewable file but they are usually not available
>>> on Windows and other non-X platforms AFAIK.
>>>
>>>
>>>   
>> Using XBM is fine (it's easily editable in either gimp or emacs) and you
>> should be able to
>> #include "leaf1.xbm"
>> 
>
> That would work if XBM and grub did not have opposite bit order. The
> bytes are ordered the same but the bits are reversed.
>
>   
 Could you split addition of videotests from the rest of the patch?
 
>
> Yes, there should be no problem with that.
>
>   
 -unsigned int x;
 -unsigned int y;
 -unsigned int width;
 -unsigned int height;
 +int x;
 +int y;
 +int width;
 +int height;
 Why do you need negative values?

 
>>> I don't need the negative values everywhere but this change was to
>>> align the typing so that casts are not required.
>>>
>>>
>>>   
>> Perhaps few casts together with appropiate checks when casting is better
>> than potentially exposing the whole code to the values it's not intended
>> for.
>> 
>
> How is it exposed to more unwanted values than now?
> It uses the variables only to store the viewport dimensions and at
> most adds a border around it. The unsigned integer is able to
> represent values outside of the viewport as much as signed integers
> are, only signed integer can represent values outside of viewport in
> both directions. The unwanted values (if erroneously calculated which
> does not seem to be the case here) are clipped by the viewport
> clipping in video_fb.
>
> On the other hand, there would be more than a few casts as the
> variables are used multiple times and the interface now uses signed
> integers.
>
>   
 +/* Supported operations are simple and easy to understand.
 + * MIRROR  |  swap image across (around) the vertical axis
 + * FLIP-  swap image across the horizontal axis - upside down
 + * SWAP/  swap image across the x=y axis - swap the x and y 
 coordinates
 It's just a D_8 group. Could you add a comment to functions what they do
 in group theoretical sense? It would make the code easier to follow and

 
>>> Obviously it is a group,
>>>   
>>> any set of transforms closed on composition is.
>>>
>>>
>>>   
>> Not true. Consider an example of empty set: it has no identity element.
>> Less trivial counter-example: set of transformations: (x,y)->(kx,ky),
>> 0> with <=1 you will have an identity element but no other element is
>> invertible.
>> 
>
> It depends on your exact definition of group which somewhat varies but
> it is true that most definitions at least require the set to be
> non-empty.
>
>   
>>> If you have clearer explanation or more efficient code please share.
>>>
>>>
>>>   
>> I was thinking of using 2 generators instead of 3:
>> 1) standard generators: s=rot90 or rot270, t=any reflection. Then all
>> elements are s^k or s^k*t. It makes computation of composition easier.
>> Rules on generators are:
>> s^n=t^2=1
>> tst=s^{-1}
>> 2) or using 2 reflections. E.g. u=| and v=/. You already figured how to
>> generate with u=|, v=/, w=-
>> But w=uvuv
>> Then rules of computation are:
>> u^2=v^2=(uv)^4=1
>> 
>
> This might be mathematically optimal but how that maps to actual efficient 
> code?
>
> In my view the v (and s) operations are expensive so I want to avoid
> them if possible.
>   
I actualy though of using preprocessor magic to make 8 blitting functions.
I don't insist on any particular group representation, just want to be
sure you take it into account.
> This requires that both u and w be in the chosen set of generators
> because otherwise use of v or s twice is required to get one from the
> other. Using (u or w) and v as the two basic operations additionally
> requires composing generators in non-fixed order to get all the 8
> possible combinations.
>
> The other reason for having 3 generators is clear and efficient
> reperesentation 

Re: [patch] GRUB possible patches

2010-02-16 Thread Vladimir 'φ-coder/phcoder' Serbinenko
gbura...@gmail.com wrote:
> Hello, 
>
>   
>> It is. Can you write a ChangeLog entry?
>> 
> Done
>
>   
First part applied
>> ;1 is so called version. Basically all terminating  ; have to be
>> stripped but only if filename doesn't come from rockridge or joliet.
>> Apparently nobody uses grub2 on non-rockridge, non-joliet iso. While
>> this bug should be fixed using grub from plain iso isn't supported due
>> to filename length limitation.
>> 
>
> I found out that I got versioning even on joliet CD. I created CD using 
> "Small CD Writer". You can try the same. I created patch for that as well, 
> but of course you can skip it.
>   
Please don't put unrelated patches into single file. Fast look on the
joliet.c of mkisofs suggests that it's not the case for its joliet. It
seems there is more to it. Can you find some normative documents?
> Regards,
> Georgy
>
> PS: My Patch
>
> === modified file 'ChangeLog'
> --- ChangeLog 2010-02-10 19:27:12 +
> +++ ChangeLog 2010-02-12 14:14:46 +
> @@ -1,3 +1,8 @@
> +2010-02-12  Georgy Buranov 
> +
> + * disk/efi/efidisk.c (grub_efidisk_get_device_name): Fix bug obtaining 
> device name of the whole disk
> + * fs/iso9660.c (grub_iso9660_iterate_dir): Remove file versions from 
> ISO9660&Joliet file names
> +
>  2010-02-10  Vladimir Serbinenko  
>  
>   Pass SIMPLE framebuffer size in bytes and not 64K blocks.
>
> === modified file 'disk/efi/efidisk.c'
> --- disk/efi/efidisk.c2010-01-20 08:12:47 +
> +++ disk/efi/efidisk.c2010-02-12 14:05:23 +
> @@ -825,7 +825,7 @@
> if (! disk)
>   return 1;
>  
> -   if (disk->id == GRUB_DISK_DEVICE_EFIDISK_ID)
> +   if (disk->dev->id == GRUB_DISK_DEVICE_EFIDISK_ID)
>   {
> struct grub_efidisk_data *d;
>  
>
> === modified file 'fs/iso9660.c'
> --- fs/iso9660.c  2010-01-27 03:11:20 +
> +++ fs/iso9660.c  2010-02-12 14:13:01 +
> @@ -615,9 +615,6 @@
>   if (!filename)
> {
>   name[dirent.namelen] = '\0';
> - filename = grub_strrchr (name, ';');
> - if (filename)
> -   *filename = '\0';
>  
>   if (dirent.namelen == 1 && name[0] == 0)
> filename = ".";
> @@ -640,6 +637,14 @@
>  
>  filename_alloc = 1;
>}
> +   
> +  if (!dir->data->rockridge)
> +  {
> +// On simple ISO 9660 disks and on joliet, we need to remove file 
> version, if any
>   
We use only C-style comments
> + char* lastSymbol = grub_strrchr (filename, ';');
> + if (lastSymbol)
> +   *lastSymbol = '\0';
> +  }
>  
>   if (hook (filename, type, node))
> {
>
>
>
> --
> This message was sent on behalf of gbura...@gmail.com at openSubscriber.com
> http://www.opensubscriber.com/message/grub-devel@gnu.org/13415591.html
>
>
> ___
> 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


Contributing patches (Re: freeze announcement)

2010-02-16 Thread Vladimir 'φ-coder/phcoder' Serbinenko
gbura...@gmail.com wrote:
>> Hi,
>> trunk repository is now frozen in preparation for GRUB 1.98, which will be
>> released soon, most likely this month.
>> I leave Vladimir in charge of this freeze process.  Please ask him for
>> approval when in doubt.
>> 
>
> Hello,
> I am really newbie to the process of reviewing/comming code to GNU 
> repositaries (or, more specifically, to GRUB2). I got sevelar patches and I 
> want them to be integrated, and I really think they can be helpful, however I 
> am worried they can be lost.
>
> Can smb please explain me the process of commiting code? Can everybody to it 
> or just special people? How can I "push" my path to commit?
>
>   
Only people listed here:
https://savannah.gnu.org/project/memberlist.php?group=grub have commit
access. Patches should be sent to mailing list. For big patches we ask
contributors to sign some legal papers. Now as of freeze only
non-intrusive patches are accepted. All others are either postponed or
applied to experimental which isn't affected by freeze
> Regards,
> Georgy
>
> --
> This message was sent on behalf of gbura...@gmail.com at openSubscriber.com
> http://www.opensubscriber.com/message/grub-devel@gnu.org/13434141.html
>
>
> ___
> 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: freeze announcement

2010-02-16 Thread gburanov
> Hi,
> trunk repository is now frozen in preparation for GRUB 1.98, which will be
> released soon, most likely this month.
> I leave Vladimir in charge of this freeze process.  Please ask him for
> approval when in doubt.

Hello,
I am really newbie to the process of reviewing/comming code to GNU repositaries 
(or, more specifically, to GRUB2). I got sevelar patches and I want them to be 
integrated, and I really think they can be helpful, however I am worried they 
can be lost.

Can smb please explain me the process of commiting code? Can everybody to it or 
just special people? How can I "push" my path to commit?

Regards,
Georgy

--
This message was sent on behalf of gbura...@gmail.com at openSubscriber.com
http://www.opensubscriber.com/message/grub-devel@gnu.org/13434141.html


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


Re: [PATCH] Trim a few bytes from boot.img

2010-02-16 Thread Vladimir 'φ-coder/phcoder' Serbinenko
Colin Watson wrote:
> A while back, I was looking for eight extra bytes in boot.img so that I
> could put a keyboard modifier check in there, allowing me to avoid
> printing "GRUB loading" by default, which my boss^3 has been asking me
> for, without crippling debugging in the process.  I knew I could find a
> few by shortening error messages (e.g. "Hard Disk" -> "HD"), but I asked
> Colin King, one of our kernel hackers, if he could find a nicer
> approach.  He came up with a patch which I massaged a bit into this.
> What do people think?
>
> It seems as if it might be generally useful to have a bit of spare space
> in here, although I would certainly appreciate it if people didn't use
> it all at once. ;-)
>   
Looks goo. Can you merge it into experimental?
> 2010-02-10  Colin King  
> 2010-02-10  Colin Watson  
>
>   Shrink the pre-partition-table part of boot.img by eight bytes.
>
>   * boot/i386/pc/boot.S (ERR): New macro.
>   (chs_mode): Use ERR.
>   (geometry_error): Likewise.
>   (hd_probe_error): Remove.  This is only used once, so we wrwite
>   it inline instead.
>   (read_error): Instead of printing read_error_string, just set up
>   %si and fall through to ...
>   (error_message): ... this new function, also used by ERR.
>
> === modified file 'boot/i386/pc/boot.S'
> --- boot/i386/pc/boot.S   2010-01-03 22:05:07 +
> +++ boot/i386/pc/boot.S   2010-02-10 11:32:08 +
> @@ -27,6 +27,7 @@
>  
>   /* Print message string */
>  #define MSG(x)   movw $x, %si; call LOCAL(message)
> +#define ERR(x)   movw $x, %si; jmp LOCAL(error_message)
>  
>   .file   "boot.S"
>  
> @@ -233,7 +234,7 @@ LOCAL(chs_mode):
>   jz  LOCAL(floppy_probe)
>  
>   /* Nope, we definitely have a hard disk, and we're screwed. */
> - jmp LOCAL(hd_probe_error)
> + ERR(hd_probe_error_string)
>  
>  LOCAL(final_init):
>   /* set the mode to zero */
> @@ -360,22 +361,15 @@ LOCAL(copy_buffer):
>   * BIOS Geometry translation error (past the end of the disk geometry!).
>   */
>  LOCAL(geometry_error):
> - MSG(geometry_error_string)
> - jmp LOCAL(general_error)
> -
> -/*
> - * Disk probe failure.
> - */
> -LOCAL(hd_probe_error):
> - MSG(hd_probe_error_string)
> - jmp LOCAL(general_error)
> + ERR(geometry_error_string)
>  
>  /*
>   * Read error on the disk.
>   */
>  LOCAL(read_error):
> - MSG(read_error_string)
> -
> + movw$read_error_string, %si
> +LOCAL(error_message):
> + callLOCAL(message)
>  LOCAL(general_error):
>   MSG(general_error_string)
>  
>
> 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: What's the point of allocating the protected mode code on 0x100000 only (UEFI)

2010-02-16 Thread Vladimir 'φ-coder/phcoder' Serbinenko
George Buranov wrote:
> Hello,
>
> I just found another computer with working UEFI (I was testing on UEFI
> simulation) and just found that the kernel load does not work. I found
> out that the problem is in code
>
>   /* Next, find free pages for the protected mode code.  */
>   /* XXX what happens if anything is using this address?  */
>   prot_mode_mem = grub_efi_allocate_pages (0x10, prot_mode_pages + 1);
>   if (! prot_mode_mem)
> {
>   grub_error (GRUB_ERR_OUT_OF_MEMORY,
>   "cannot allocate protected mode pages");
>   goto fail;
> }
>
> This function return with false for me. I am continuing investigation,
> but maybe you know the reason why the allocation is on this special
> address? Can we change it?
As I already said I have a working prototype for this in newreloc branch.
>
> Regards,
> Georgy
> 
>
> ___
> 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


What's the point of allocating the protected mode code on 0x100000 only (UEFI)

2010-02-16 Thread George Buranov
Hello,

I just found another computer with working UEFI (I was testing on UEFI
simulation) and just found that the kernel load does not work. I found out
that the problem is in code

  /* Next, find free pages for the protected mode code.  */
  /* XXX what happens if anything is using this address?  */
  prot_mode_mem = grub_efi_allocate_pages (0x10, prot_mode_pages + 1);
  if (! prot_mode_mem)
{
  grub_error (GRUB_ERR_OUT_OF_MEMORY,
  "cannot allocate protected mode pages");
  goto fail;
}

This function return with false for me. I am continuing investigation, but
maybe you know the reason why the allocation is on this special address? Can
we change it?

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