Re: Re: [patch] GRUB possible patches
> 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)
> 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
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
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
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
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
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
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
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
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
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
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/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/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
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
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
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
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
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
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
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
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
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
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
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/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/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
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/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
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
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?
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?
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
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
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
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)
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
> 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
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)
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)
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