[U-Boot] [PATCH v2 0/2] ext2: Cleanup and simplify sector access code
This patch set first cleans up all of the checkpatch warnings and errors in fs/ext2/dev.c and then cleans up the partial sector access logic in the ext2fs_devread function.
I didn't see a file system or ext2 custodian so I've CC'ed Andy the custodian for MMC as that seems closest. Please let me know if I should bring this to someone elses attention.
changes in v2 - Update commit message to include mention of printf and syle cleanup
Signed-off-by: Anton Staaf robotboy@chromium.org Cc: Andy Fleming afleming@freescale.com Cc: Detlev Zundel dzu@denx.de
Anton Staaf (2): ext2: Fix checkpatch violations ext2: Simplify partial sector access logic
fs/ext2/dev.c | 112 ++++++++++++++++++++++++++------------------------------ 1 files changed, 52 insertions(+), 60 deletions(-)
Fix all checkpatch violations in the low level Ext2 block device reading code. This is done in preparation for cleaning up the partial sector access code.
Also replace hard coded function names in printfs with __func__ macro, and correctly indent comments for style consistency not captured by checkpatch.
Signed-off-by: Anton Staaf robotboy@chromium.org Cc: Andy Fleming afleming@freescale.com Cc: Detlev Zundel dzu@denx.de --- fs/ext2/dev.c | 82 ++++++++++++++++++++++++++++++--------------------------- 1 files changed, 43 insertions(+), 39 deletions(-)
diff --git a/fs/ext2/dev.c b/fs/ext2/dev.c index 3b49650..4365b3b 100644 --- a/fs/ext2/dev.c +++ b/fs/ext2/dev.c @@ -31,7 +31,7 @@ static block_dev_desc_t *ext2fs_block_dev_desc; static disk_partition_t part_info;
-int ext2fs_set_blk_dev (block_dev_desc_t * rbdd, int part) +int ext2fs_set_blk_dev(block_dev_desc_t *rbdd, int part) { ext2fs_block_dev_desc = rbdd;
@@ -46,51 +46,55 @@ int ext2fs_set_blk_dev (block_dev_desc_t * rbdd, int part) return 0; } } - return (part_info.size); + return part_info.size; }
-int ext2fs_devread (int sector, int byte_offset, int byte_len, char *buf) { +int ext2fs_devread(int sector, int byte_offset, int byte_len, char *buf) +{ char sec_buf[SECTOR_SIZE]; unsigned block_len;
-/* - * Check partition boundaries - */ - if ((sector < 0) - || ((sector + ((byte_offset + byte_len - 1) >> SECTOR_BITS)) >= + /* + * Check partition boundaries + */ + if ((sector < 0) || + ((sector + ((byte_offset + byte_len - 1) >> SECTOR_BITS)) >= part_info.size)) { - /* errnum = ERR_OUTSIDE_PART; */ - printf (" ** ext2fs_devread() read outside partition sector %d\n", sector); - return (0); + /* errnum = ERR_OUTSIDE_PART; */ + printf(" ** %s read outside partition sector %d\n", + __func__, + sector); + return 0; }
-/* - * Get the read to the beginning of a partition. - */ + /* + * Get the read to the beginning of a partition. + */ sector += byte_offset >> SECTOR_BITS; byte_offset &= SECTOR_SIZE - 1;
- debug (" <%d, %d, %d>\n", sector, byte_offset, byte_len); + debug(" <%d, %d, %d>\n", sector, byte_offset, byte_len);
if (ext2fs_block_dev_desc == NULL) { - printf ("** Invalid Block Device Descriptor (NULL)\n"); - return (0); + printf(" ** %s Invalid Block Device Descriptor (NULL)\n", + __func__); + return 0; }
if (byte_offset != 0) { /* read first part which isn't aligned with start of sector */ if (ext2fs_block_dev_desc-> - block_read (ext2fs_block_dev_desc->dev, - part_info.start + sector, 1, - (unsigned long *) sec_buf) != 1) { - printf (" ** ext2fs_devread() read error **\n"); - return (0); + block_read(ext2fs_block_dev_desc->dev, + part_info.start + sector, 1, + (unsigned long *) sec_buf) != 1) { + printf(" ** %s read error **\n", __func__); + return 0; } - memcpy (buf, sec_buf + byte_offset, - min (SECTOR_SIZE - byte_offset, byte_len)); - buf += min (SECTOR_SIZE - byte_offset, byte_len); - byte_len -= min (SECTOR_SIZE - byte_offset, byte_len); + memcpy(buf, sec_buf + byte_offset, + min(SECTOR_SIZE - byte_offset, byte_len)); + buf += min(SECTOR_SIZE - byte_offset, byte_len); + byte_len -= min(SECTOR_SIZE - byte_offset, byte_len); sector++; }
@@ -111,13 +115,13 @@ int ext2fs_devread (int sector, int byte_offset, int byte_len, char *buf) { return 1; }
- if (ext2fs_block_dev_desc->block_read (ext2fs_block_dev_desc->dev, - part_info.start + sector, - block_len / SECTOR_SIZE, - (unsigned long *) buf) != + if (ext2fs_block_dev_desc->block_read(ext2fs_block_dev_desc->dev, + part_info.start + sector, + block_len / SECTOR_SIZE, + (unsigned long *) buf) != block_len / SECTOR_SIZE) { - printf (" ** ext2fs_devread() read error - block\n"); - return (0); + printf(" ** %s read error - block\n", __func__); + return 0; } block_len = byte_len & ~(SECTOR_SIZE - 1); buf += block_len; @@ -127,13 +131,13 @@ int ext2fs_devread (int sector, int byte_offset, int byte_len, char *buf) { if (byte_len != 0) { /* read rest of data which are not in whole sector */ if (ext2fs_block_dev_desc-> - block_read (ext2fs_block_dev_desc->dev, - part_info.start + sector, 1, - (unsigned long *) sec_buf) != 1) { - printf (" ** ext2fs_devread() read error - last part\n"); - return (0); + block_read(ext2fs_block_dev_desc->dev, + part_info.start + sector, 1, + (unsigned long *) sec_buf) != 1) { + printf(" ** %s read error - last part\n", __func__); + return 0; } - memcpy (buf, sec_buf, byte_len); + memcpy(buf, sec_buf, byte_len); } - return (1); + return 1; }
Hi Anton,
Fix all checkpatch violations in the low level Ext2 block device reading code. This is done in preparation for cleaning up the partial sector access code.
Also replace hard coded function names in printfs with __func__ macro, and correctly indent comments for style consistency not captured by checkpatch.
Signed-off-by: Anton Staaf robotboy@chromium.org Cc: Andy Fleming afleming@freescale.com Cc: Detlev Zundel dzu@denx.de
Thanks for the re-spin.
Acked-by: Detlev Zundel dzu@denx.de
Dear Anton Staaf,
In message 1309459537-312-2-git-send-email-robotboy@chromium.org you wrote:
Fix all checkpatch violations in the low level Ext2 block device reading code. This is done in preparation for cleaning up the partial sector access code.
Also replace hard coded function names in printfs with __func__ macro, and correctly indent comments for style consistency not captured by checkpatch.
Signed-off-by: Anton Staaf robotboy@chromium.org Cc: Andy Fleming afleming@freescale.com Cc: Detlev Zundel dzu@denx.de
fs/ext2/dev.c | 82 ++++++++++++++++++++++++++++++--------------------------- 1 files changed, 43 insertions(+), 39 deletions(-)
Applied, thanks.
Best regards,
Wolfgang Denk
Previously reading zero full sectors (reading the end of one sector and the beginning of the next for example) was special cased and involved stack allocating a second sector buffer. This change uses the same code path for this case as well as when there are a non-zero number of full sectors to access. The result is easier to read and reduces the maximum stack used.
Signed-off-by: Anton Staaf robotboy@chromium.org Cc: Andy Fleming afleming@freescale.com Cc: Detlev Zundel dzu@denx.de --- fs/ext2/dev.c | 42 +++++++++++++++--------------------------- 1 files changed, 15 insertions(+), 27 deletions(-)
diff --git a/fs/ext2/dev.c b/fs/ext2/dev.c index 4365b3b..78851d0 100644 --- a/fs/ext2/dev.c +++ b/fs/ext2/dev.c @@ -53,7 +53,7 @@ int ext2fs_set_blk_dev(block_dev_desc_t *rbdd, int part) int ext2fs_devread(int sector, int byte_offset, int byte_len, char *buf) { char sec_buf[SECTOR_SIZE]; - unsigned block_len; + unsigned sectors;
/* * Check partition boundaries @@ -98,35 +98,23 @@ int ext2fs_devread(int sector, int byte_offset, int byte_len, char *buf) sector++; }
- if (byte_len == 0) - return 1; - /* read sector aligned part */ - block_len = byte_len & ~(SECTOR_SIZE - 1); - - if (block_len == 0) { - u8 p[SECTOR_SIZE]; - - block_len = SECTOR_SIZE; - ext2fs_block_dev_desc->block_read(ext2fs_block_dev_desc->dev, - part_info.start + sector, - 1, (unsigned long *)p); - memcpy(buf, p, byte_len); - return 1; - } + sectors = byte_len / SECTOR_SIZE; + + if (sectors > 0) { + if (ext2fs_block_dev_desc->block_read( + ext2fs_block_dev_desc->dev, + part_info.start + sector, + sectors, + (unsigned long *) buf) != sectors) { + printf(" ** %s read error - block\n", __func__); + return 0; + }
- if (ext2fs_block_dev_desc->block_read(ext2fs_block_dev_desc->dev, - part_info.start + sector, - block_len / SECTOR_SIZE, - (unsigned long *) buf) != - block_len / SECTOR_SIZE) { - printf(" ** %s read error - block\n", __func__); - return 0; + buf += sectors * SECTOR_SIZE; + byte_len -= sectors * SECTOR_SIZE; + sector += sectors; } - block_len = byte_len & ~(SECTOR_SIZE - 1); - buf += block_len; - byte_len -= block_len; - sector += block_len / SECTOR_SIZE;
if (byte_len != 0) { /* read rest of data which are not in whole sector */
Hi Anton,
Previously reading zero full sectors (reading the end of one sector and the beginning of the next for example) was special cased and involved stack allocating a second sector buffer. This change uses the same code path for this case as well as when there are a non-zero number of full sectors to access. The result is easier to read and reduces the maximum stack used.
Signed-off-by: Anton Staaf robotboy@chromium.org Cc: Andy Fleming afleming@freescale.com Cc: Detlev Zundel dzu@denx.de
Acked-by: Detlev Zundel dzu@denx.de
Dear Anton Staaf,
In message 1309459537-312-3-git-send-email-robotboy@chromium.org you wrote:
Previously reading zero full sectors (reading the end of one sector and the beginning of the next for example) was special cased and involved stack allocating a second sector buffer. This change uses the same code path for this case as well as when there are a non-zero number of full sectors to access. The result is easier to read and reduces the maximum stack used.
Signed-off-by: Anton Staaf robotboy@chromium.org Cc: Andy Fleming afleming@freescale.com Cc: Detlev Zundel dzu@denx.de
fs/ext2/dev.c | 42 +++++++++++++++--------------------------- 1 files changed, 15 insertions(+), 27 deletions(-)
Applied, thanks.
Best regards,
Wolfgang Denk
participants (3)
-
Anton Staaf -
Detlev Zundel -
Wolfgang Denk