| Message ID | 1390507020-15766-5-git-send-email-swarren@wwwdotorg.org |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
Hi Stephen, On 23 January 2014 12:57, Stephen Warren <swarren@wwwdotorg.org> wrote: > From: Stephen Warren <swarren@nvidia.com> > > This hooks into the generic "file exists" support added in an earlier > patch, and provides an implementation for the ext4 filesystem. > s/ext4/fat > > Signed-off-by: Stephen Warren <swarren@nvidia.com> > --- > v2: s/ext/fat/ in the commit subject > --- > fs/fat/fat.c | 18 ++++++++++++++---- > fs/fs.c | 2 +- > include/fat.h | 1 + > 3 files changed, 16 insertions(+), 5 deletions(-) > > diff --git a/fs/fat/fat.c b/fs/fat/fat.c > index b41d62e3c386..bc06c0a3c688 100644 > --- a/fs/fat/fat.c > +++ b/fs/fat/fat.c > @@ -808,7 +808,7 @@ __u8 do_fat_read_at_block[MAX_CLUSTSIZE] > > long > do_fat_read_at(const char *filename, unsigned long pos, void *buffer, > - unsigned long maxsize, int dols) > + unsigned long maxsize, int dols, int dogetsize) > I think it would be better to combine the three available operations (read, ls and getsize) into an enum and pass the required operation explicitly into this function in a single parameter. > { > char fnamecopy[2048]; > boot_sector bs; > @@ -1152,7 +1152,10 @@ rootdir_done: > subname = nextname; > } > > - ret = get_contents(mydata, dentptr, pos, buffer, maxsize); > + if (dogetsize) > + ret = FAT2CPU32(dentptr->size); > + else > + Doesn't this mean you are actually reading the contents here into a NULL buffer? At least I think it needs a comment as to why this works. > ret = get_contents(mydata, dentptr, pos, buffer, maxsize); > debug("Size: %d, got: %ld\n", FAT2CPU32(dentptr->size), ret); > > exit: > @@ -1163,7 +1166,7 @@ exit: > long > do_fat_read(const char *filename, void *buffer, unsigned long maxsize, > int dols) > { > - return do_fat_read_at(filename, 0, buffer, maxsize, dols); > + return do_fat_read_at(filename, 0, buffer, maxsize, dols, 0); > } > > int file_fat_detectfs(void) > @@ -1233,11 +1236,18 @@ int file_fat_ls(const char *dir) > return do_fat_read(dir, NULL, 0, LS_YES); > } > > +int fat_exists(const char *filename) > +{ > + int sz; > + sz = do_fat_read_at(filename, 0, NULL, 0, LS_NO, 1); > + return (sz >= 0) ? 0 : 1; > +} > + > long file_fat_read_at(const char *filename, unsigned long pos, void > *buffer, > unsigned long maxsize) > { > printf("reading %s\n", filename); > - return do_fat_read_at(filename, pos, buffer, maxsize, LS_NO); > + return do_fat_read_at(filename, pos, buffer, maxsize, LS_NO, 0); > } > > long file_fat_read(const char *filename, void *buffer, unsigned long > maxsize) > diff --git a/fs/fs.c b/fs/fs.c > index 3f14d0169b43..d2bc8d0f1459 100644 > --- a/fs/fs.c > +++ b/fs/fs.c > @@ -80,7 +80,7 @@ static struct fstype_info fstypes[] = { > .probe = fat_set_blk_dev, > .close = fat_close, > .ls = file_fat_ls, > - .exists = fs_exists_unsupported, > + .exists = fat_exists, > .read = fat_read_file, > .write = fs_write_unsupported, > }, > diff --git a/include/fat.h b/include/fat.h > index 2c951e7d79c6..c8eb7ccd2904 100644 > --- a/include/fat.h > +++ b/include/fat.h > @@ -188,6 +188,7 @@ file_read_func file_fat_read; > int file_cd(const char *path); > int file_fat_detectfs(void); > int file_fat_ls(const char *dir); > +int fat_exists(const char *filename); > long file_fat_read_at(const char *filename, unsigned long pos, void > *buffer, > unsigned long maxsize); > long file_fat_read(const char *filename, void *buffer, unsigned long > maxsize); > -- > 1.8.1.5 > > Regards, Simon
On 01/26/2014 08:52 AM, Simon Glass wrote: > Hi Stephen, > > On 23 January 2014 12:57, Stephen Warren <swarren@wwwdotorg.org > <mailto:swarren@wwwdotorg.org>> wrote: > > From: Stephen Warren <swarren@nvidia.com <mailto:swarren@nvidia.com>> > > This hooks into the generic "file exists" support added in an earlier > patch, and provides an implementation for the ext4 filesystem. > diff --git a/fs/fat/fat.c b/fs/fat/fat.c > @@ -808,7 +808,7 @@ __u8 do_fat_read_at_block[MAX_CLUSTSIZE] > > long > do_fat_read_at(const char *filename, unsigned long pos, void *buffer, > - unsigned long maxsize, int dols) > + unsigned long maxsize, int dols, int dogetsize) > > I think it would be better to combine the three available operations > (read, ls and getsize) into an enum and pass the required operation > explicitly into this function in a single parameter. I had originally thought that, but the implementation of the function changes the value of dols during operation. I suppose it would be possible to achieve that using bit-mask operations on the variable, but it seemed a bit cleaner to just make it a separate variable rather than using fields within a somewhat unrelated variable. Would you still prefer to combine them into one:? > - ret = get_contents(mydata, dentptr, pos, buffer, maxsize); > + if (dogetsize) > + ret = FAT2CPU32(dentptr->size); > + else > + > > > Doesn't this mean you are actually reading the contents here into a NULL > buffer? At least I think it needs a comment as to why this works. > > ret = get_contents(mydata, dentptr, pos, buffer, maxsize); > debug("Size: %d, got: %ld\n", FAT2CPU32(dentptr->size), ret); get_contents() is the function that actually reads the file content; everything before this point is simply parsing the directory entry for the file. So, no, I don't think the file content is read at all. That implementation of dols!=0 is somewhat similar, although it does "goto exit" at a different location in the code.
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index b41d62e3c386..bc06c0a3c688 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -808,7 +808,7 @@ __u8 do_fat_read_at_block[MAX_CLUSTSIZE] long do_fat_read_at(const char *filename, unsigned long pos, void *buffer, - unsigned long maxsize, int dols) + unsigned long maxsize, int dols, int dogetsize) { char fnamecopy[2048]; boot_sector bs; @@ -1152,7 +1152,10 @@ rootdir_done: subname = nextname; } - ret = get_contents(mydata, dentptr, pos, buffer, maxsize); + if (dogetsize) + ret = FAT2CPU32(dentptr->size); + else + ret = get_contents(mydata, dentptr, pos, buffer, maxsize); debug("Size: %d, got: %ld\n", FAT2CPU32(dentptr->size), ret); exit: @@ -1163,7 +1166,7 @@ exit: long do_fat_read(const char *filename, void *buffer, unsigned long maxsize, int dols) { - return do_fat_read_at(filename, 0, buffer, maxsize, dols); + return do_fat_read_at(filename, 0, buffer, maxsize, dols, 0); } int file_fat_detectfs(void) @@ -1233,11 +1236,18 @@ int file_fat_ls(const char *dir) return do_fat_read(dir, NULL, 0, LS_YES); } +int fat_exists(const char *filename) +{ + int sz; + sz = do_fat_read_at(filename, 0, NULL, 0, LS_NO, 1); + return (sz >= 0) ? 0 : 1; +} + long file_fat_read_at(const char *filename, unsigned long pos, void *buffer, unsigned long maxsize) { printf("reading %s\n", filename); - return do_fat_read_at(filename, pos, buffer, maxsize, LS_NO); + return do_fat_read_at(filename, pos, buffer, maxsize, LS_NO, 0); } long file_fat_read(const char *filename, void *buffer, unsigned long maxsize) diff --git a/fs/fs.c b/fs/fs.c index 3f14d0169b43..d2bc8d0f1459 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -80,7 +80,7 @@ static struct fstype_info fstypes[] = { .probe = fat_set_blk_dev, .close = fat_close, .ls = file_fat_ls, - .exists = fs_exists_unsupported, + .exists = fat_exists, .read = fat_read_file, .write = fs_write_unsupported, }, diff --git a/include/fat.h b/include/fat.h index 2c951e7d79c6..c8eb7ccd2904 100644 --- a/include/fat.h +++ b/include/fat.h @@ -188,6 +188,7 @@ file_read_func file_fat_read; int file_cd(const char *path); int file_fat_detectfs(void); int file_fat_ls(const char *dir); +int fat_exists(const char *filename); long file_fat_read_at(const char *filename, unsigned long pos, void *buffer, unsigned long maxsize); long file_fat_read(const char *filename, void *buffer, unsigned long maxsize);