On Mon, Jul 28, 2008 at 3:51 AM, Colin D Bennett <[EMAIL PROTECTED]> wrote: > On Mon, 28 Jul 2008 01:52:40 +0800 > Bean <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> >> >> * 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.
Hi, Oh, I just spot a few bugs, please try the new patch. -- Bean
diff --git a/conf/common.rmk b/conf/common.rmk index 7db0b2a..3d3cd8a 100644 --- a/conf/common.rmk +++ b/conf/common.rmk @@ -382,7 +382,7 @@ crc_mod_CFLAGS = $(COMMON_CFLAGS) crc_mod_LDFLAGS = $(COMMON_LDFLAGS) # Misc. -pkglib_MODULES += gzio.mod elf.mod +pkglib_MODULES += gzio.mod bufio.mod elf.mod # For elf.mod. elf_mod_SOURCES = kern/elf.c @@ -393,3 +393,8 @@ elf_mod_LDFLAGS = $(COMMON_LDFLAGS) gzio_mod_SOURCES = io/gzio.c gzio_mod_CFLAGS = $(COMMON_CFLAGS) gzio_mod_LDFLAGS = $(COMMON_LDFLAGS) + +# For bufio.mod. +bufio_mod_SOURCES = io/bufio.c +bufio_mod_CFLAGS = $(COMMON_CFLAGS) +bufio_mod_LDFLAGS = $(COMMON_LDFLAGS) diff --git a/include/grub/bufio.h b/include/grub/bufio.h new file mode 100644 index 0000000..5ca54dd --- /dev/null +++ b/include/grub/bufio.h @@ -0,0 +1,28 @@ +/* bufio.h - prototypes for bufio */ +/* + * GRUB -- GRand Unified Bootloader + * Copyright (C) 2008 Free Software Foundation, Inc. + * + * GRUB is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * GRUB is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with GRUB. If not, see <http://www.gnu.org/licenses/>. + */ + +#ifndef GRUB_BUFIO_H +#define GRUB_BUFIO_H 1 + +#include <grub/file.h> + +grub_file_t grub_bufio_open (grub_file_t io); +grub_file_t grub_buffile_open (const char *name); + +#endif /* ! GRUB_BUFIO_H */ diff --git a/io/bufio.c b/io/bufio.c new file mode 100644 index 0000000..b1b2a2f --- /dev/null +++ b/io/bufio.c @@ -0,0 +1,174 @@ +/* bufio.c - buffered io access */ +/* + * GRUB -- GRand Unified Bootloader + * Copyright (C) 2008 Free Software Foundation, Inc. + * + * GRUB is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * GRUB is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with GRUB. If not, see <http://www.gnu.org/licenses/>. + */ + +#include <grub/err.h> +#include <grub/types.h> +#include <grub/mm.h> +#include <grub/misc.h> +#include <grub/fs.h> +#include <grub/bufio.h> + +#define GRUB_BUFIO_BUFSIZE 8192 + +struct grub_bufio +{ + grub_file_t file; + grub_size_t length; + char buffer[GRUB_BUFIO_BUFSIZE]; +}; +typedef struct grub_bufio *grub_bufio_t; + +static struct grub_fs grub_bufio_fs; + +grub_file_t +grub_bufio_open (grub_file_t io) +{ + grub_file_t file; + grub_bufio_t bufio = 0; + + file = (grub_file_t) grub_malloc (sizeof (*file)); + if (! file) + return 0; + + bufio = grub_malloc (sizeof (*bufio)); + if (! bufio) + { + grub_free (file); + return 0; + } + + grub_memset (bufio, 0, sizeof (*bufio)); + bufio->file = io; + bufio->file->offset = -1; + + file->device = io->device; + file->offset = 0; + file->size = io->size; + file->data = bufio; + file->read_hook = 0; + file->fs = &grub_bufio_fs; + + return file; +} + +grub_file_t +grub_buffile_open (const char *name) +{ + grub_file_t io, file; + + io = grub_file_open (name); + if (! io) + return 0; + + file = grub_bufio_open (io); + if (! file) + { + grub_file_close (io); + return 0; + } + + return file; +} + +static grub_ssize_t +grub_bufio_read (grub_file_t file, char *buf, grub_size_t len) +{ + grub_size_t res = len; + grub_bufio_t bufio = file->data; + grub_size_t pos = file->offset & (GRUB_BUFIO_BUFSIZE - 1); + + if (bufio->file->offset == file->offset - pos) + { + grub_size_t n; + + n = bufio->length - pos; + if (n > len) + n = len; + + grub_memcpy (buf, &bufio->buffer[pos], n); + len -= n; + + if (! len) + return res; + + buf += n; + pos = 0; + bufio->file->offset += bufio->length; + } + else + bufio->file->offset = file->offset - pos; + + while (pos + len >= GRUB_BUFIO_BUFSIZE) + { + grub_size_t n; + + n = GRUB_BUFIO_BUFSIZE - pos; + bufio->file->offset += pos; + bufio->file->fs->read (bufio->file, buf, n); + if (grub_errno) + return -1; + + buf += n; + len -= n; + bufio->file->offset += n; + pos = 0; + } + + if (! len) + { + bufio->file->offset = -1; + return res; + } + + bufio->length = bufio->file->size - bufio->file->offset; + if (bufio->length > GRUB_BUFIO_BUFSIZE) + bufio->length = GRUB_BUFIO_BUFSIZE; + + bufio->file->fs->read (bufio->file, bufio->buffer, bufio->length); + if (grub_errno) + return -1; + + grub_memcpy (buf, &bufio->buffer[pos], len); + + return res; +} + +static grub_err_t +grub_bufio_close (grub_file_t file) +{ + grub_bufio_t bufio = file->data; + + grub_file_close (bufio->file); + grub_free (bufio); + + file->device = 0; + + return grub_errno; +} + +static struct grub_fs grub_bufio_fs = + { + .name = "bufio", + .dir = 0, + .open = 0, + .read = grub_bufio_read, + .close = grub_bufio_close, + .label = 0, + .next = 0 + }; diff --git a/video/readers/jpeg.c b/video/readers/jpeg.c index 8a4f803..bec084e 100644 --- a/video/readers/jpeg.c +++ b/video/readers/jpeg.c @@ -23,7 +23,7 @@ #include <grub/mm.h> #include <grub/misc.h> #include <grub/arg.h> -#include <grub/file.h> +#include <grub/bufio.h> /* Uncomment following define to enable JPEG debug. */ //#define JPEG_DEBUG @@ -664,7 +664,7 @@ grub_video_reader_jpeg (struct grub_video_bitmap **bitmap, grub_file_t file; struct grub_jpeg_data *data; - file = grub_file_open (filename); + file = grub_buffile_open (filename); if (!file) return grub_errno; diff --git a/video/readers/png.c b/video/readers/png.c index 9dac4b6..ce38f9f 100644 --- a/video/readers/png.c +++ b/video/readers/png.c @@ -23,7 +23,7 @@ #include <grub/mm.h> #include <grub/misc.h> #include <grub/arg.h> -#include <grub/file.h> +#include <grub/bufio.h> /* Uncomment following define to enable PNG debug. */ //#define PNG_DEBUG @@ -840,7 +840,7 @@ grub_video_reader_png (struct grub_video_bitmap **bitmap, grub_file_t file; struct grub_png_data *data; - file = grub_file_open (filename); + file = grub_buffile_open (filename); if (!file) return grub_errno; diff --git a/video/readers/tga.c b/video/readers/tga.c index 7b944b0..0fe785f 100644 --- a/video/readers/tga.c +++ b/video/readers/tga.c @@ -23,7 +23,7 @@ #include <grub/mm.h> #include <grub/misc.h> #include <grub/arg.h> -#include <grub/file.h> +#include <grub/bufio.h> /* Uncomment following define to enable TGA debug. */ //#define TGA_DEBUG @@ -319,7 +319,7 @@ grub_video_reader_tga (struct grub_video_bitmap **bitmap, struct grub_tga_header header; int has_alpha; - file = grub_file_open (filename); + file = grub_buffile_open (filename); if (! file) return grub_errno;
_______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
