Re: [PATCH] buffered file read

[Colin D Bennett]

On Mon, 28 Jul 2008 01:52:40 +0800
Bean <address@hidden> wrote:

> Hi,
> 
> This patch add a new module bufio, which reads block of data at a
> time, and return the required range to the upper level. This is
> extremely useful in modules like png, where data are normally read one
> byte at a time.
> 
> To use buffered io service, just include header file <grub/bufio.h>,
> and replace grub_file_open with grub_buffile_open.
> 
> Changelog:
> 
> 2008-07-27  Bean  <address@hidden>
>       
>       * conf/common.rmk (pkglib_MODULES): Add bufio.mod.
>       (bufio_mod_SOURCES): New macro.
>       (bufio_mod_CFLAGS): Likewise.
>       (bufio_mod_LDFLAGS): Likewise.
> 
>       * include/grub/bufio.h: New file.
> 
>       * io/bufio.c: Likewise.
> 
>       * video/png.c (grub_video_reader_png): Use grub_buffile_open
> to open file.
> 
>       * video/jpeg.c (grub_video_reader_jpeg): Likewise.
> 
>       * video/tga.c (grub_video_reader_tga): Likewise.

Hi Bean,

Beautiful code!  Thanks so much for doing this work -- using buffered
file reads is absolutely necessary from a performance perspective, and
I am glad to see it implemented so cleanly.

My poor hack of grub_file_read() bows to your far superior patch!  I
really appreciate the simplicity and clarity of both its design and its
use.  I like how you implemented it using the Decorator pattern (from
the GoF design patterns) to wrap the basic file with the buffered file
layer in a way transparent to users. This is very intuitive to me,
especially since Java does exactly the same thing with InputStream and
BufferedInputStream.

The best part is that users of file I/O only have to change one
function call to get all the benefits of the buffering.

I will begin testing with this patch instead of mine shortly.

Thanks again!
Regards,
Colin