[PATCH 1/4] e2fsck: remove unused variable 'new_array'

[PATCH 1/4] e2fsck: remove unused variable 'new_array'

[Lukas Czerner]

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 e2fsck/rehash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/e2fsck/rehash.c b/e2fsck/rehash.c
index 1616d07a..b356b92d 100644
--- a/e2fsck/rehash.c
+++ b/e2fsck/rehash.c
@@ -109,7 +109,7 @@ static int fill_dir_block(ext2_filsys fs,
 			  void *priv_data)
 {
 	struct fill_dir_struct	*fd = (struct fill_dir_struct *) priv_data;
-	struct hash_entry 	*new_array, *ent;
+	struct hash_entry 	*ent;
 	struct ext2_dir_entry 	*dirent;
 	char			*dir;
 	unsigned int		offset, dir_offset, rec_len, name_len;
-- 
2.21.3

[PATCH 2/4] e2fsck: use size_t instead of int in string_copy()

[Lukas Czerner]

len argument in string_copy() is int, but it is used with malloc(),
strlen(), strncpy() and some callers use sizeof() to pass value in. So
it really ought to be size_t rather than int. Fix it.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 e2fsck/e2fsck.h | 2 +-
 e2fsck/util.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/e2fsck/e2fsck.h b/e2fsck/e2fsck.h
index 9b2b9ce8..85f953b2 100644
--- a/e2fsck/e2fsck.h
+++ b/e2fsck/e2fsck.h
@@ -627,7 +627,7 @@ extern void log_err(e2fsck_t ctx, const char *fmt, ...)
 extern void e2fsck_read_bitmaps(e2fsck_t ctx);
 extern void e2fsck_write_bitmaps(e2fsck_t ctx);
 extern void preenhalt(e2fsck_t ctx);
-extern char *string_copy(e2fsck_t ctx, const char *str, int len);
+extern char *string_copy(e2fsck_t ctx, const char *str, size_t len);
 extern int fs_proc_check(const char *fs_name);
 extern int check_for_modules(const char *fs_name);
 #ifdef RESOURCE_TRACK
diff --git a/e2fsck/util.c b/e2fsck/util.c
index d98b8e47..88e0ea8a 100644
--- a/e2fsck/util.c
+++ b/e2fsck/util.c
@@ -135,7 +135,7 @@ void *e2fsck_allocate_memory(e2fsck_t ctx, unsigned long size,
 }
 
 char *string_copy(e2fsck_t ctx EXT2FS_ATTR((unused)),
-		  const char *str, int len)
+		  const char *str, size_t len)
 {
 	char	*ret;
 
-- 
2.21.3

Re: [PATCH 2/4] e2fsck: use size_t instead of int in string_copy()

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 1997 bytes --]

On Jun 5, 2020, at 2:14 AM, Lukas Czerner <lczerner@redhat.com> wrote:
> 
> len argument in string_copy() is int, but it is used with malloc(),
> strlen(), strncpy() and some callers use sizeof() to pass value in. So
> it really ought to be size_t rather than int. Fix it.
> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>

Thanks, having good types makes it much easier to follow code logic.

There are still a bunch of places in e2fsprogs that are using "int" and
"long" for various blocks, counters, etc. that could use more precise
types.  I did a bunch of cleanup when I reviewd all of the directory and
allocation code for 64-bit issues. It still lingers elsewhere, but every
fix improves the code a bit, if we keep an eye on other incoming changes.

Reviewed-by: Andreas Dilger <adilger@dilger.ca>

> ---
> e2fsck/e2fsck.h | 2 +-
> e2fsck/util.c   | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/e2fsck/e2fsck.h b/e2fsck/e2fsck.h
> index 9b2b9ce8..85f953b2 100644
> --- a/e2fsck/e2fsck.h
> +++ b/e2fsck/e2fsck.h
> @@ -627,7 +627,7 @@ extern void log_err(e2fsck_t ctx, const char *fmt, ...)
> extern void e2fsck_read_bitmaps(e2fsck_t ctx);
> extern void e2fsck_write_bitmaps(e2fsck_t ctx);
> extern void preenhalt(e2fsck_t ctx);
> -extern char *string_copy(e2fsck_t ctx, const char *str, int len);
> +extern char *string_copy(e2fsck_t ctx, const char *str, size_t len);
> extern int fs_proc_check(const char *fs_name);
> extern int check_for_modules(const char *fs_name);
> #ifdef RESOURCE_TRACK
> diff --git a/e2fsck/util.c b/e2fsck/util.c
> index d98b8e47..88e0ea8a 100644
> --- a/e2fsck/util.c
> +++ b/e2fsck/util.c
> @@ -135,7 +135,7 @@ void *e2fsck_allocate_memory(e2fsck_t ctx, unsigned long size,
> }
> 
> char *string_copy(e2fsck_t ctx EXT2FS_ATTR((unused)),
> -		  const char *str, int len)
> +		  const char *str, size_t len)
> {
> 	char	*ret;
> 
> --
> 2.21.3
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

Re: [PATCH 2/4] e2fsck: use size_t instead of int in string_copy()

[Theodore Y. Ts'o]

On Fri, Jun 05, 2020 at 10:14:40AM +0200, Lukas Czerner wrote:
> len argument in string_copy() is int, but it is used with malloc(),
> strlen(), strncpy() and some callers use sizeof() to pass value in. So
> it really ought to be size_t rather than int. Fix it.
> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>

Thanks, applied.

					- Ted

[PATCH 3/4] e2fsck: use the right conversion specifier in e2fsck_allocate_memory()

[Lukas Czerner]

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 e2fsck/util.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/e2fsck/util.c b/e2fsck/util.c
index 88e0ea8a..79916928 100644
--- a/e2fsck/util.c
+++ b/e2fsck/util.c
@@ -123,10 +123,10 @@ void *e2fsck_allocate_memory(e2fsck_t ctx, unsigned long size,
 	char buf[256];
 
 #ifdef DEBUG_ALLOCATE_MEMORY
-	printf("Allocating %u bytes for %s...\n", size, description);
+	printf("Allocating %lu bytes for %s...\n", size, description);
 #endif
 	if (ext2fs_get_memzero(size, &ret)) {
-		sprintf(buf, "Can't allocate %u bytes for %s\n",
+		sprintf(buf, "Can't allocate %lu bytes for %s\n",
 			size, description);
 		fatal_error(ctx, buf);
 	}
-- 
2.21.3

Re: [PATCH 3/4] e2fsck: use the right conversion specifier in e2fsck_allocate_memory()

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 950 bytes --]

On Jun 5, 2020, at 2:14 AM, Lukas Czerner <lczerner@redhat.com> wrote:
> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>

Reviewed-by: Andreas Dilger <adilger@dilger.ca>

> ---
> e2fsck/util.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/e2fsck/util.c b/e2fsck/util.c
> index 88e0ea8a..79916928 100644
> --- a/e2fsck/util.c
> +++ b/e2fsck/util.c
> @@ -123,10 +123,10 @@ void *e2fsck_allocate_memory(e2fsck_t ctx, unsigned long size,
> 	char buf[256];
> 
> #ifdef DEBUG_ALLOCATE_MEMORY
> -	printf("Allocating %u bytes for %s...\n", size, description);
> +	printf("Allocating %lu bytes for %s...\n", size, description);
> #endif
> 	if (ext2fs_get_memzero(size, &ret)) {
> -		sprintf(buf, "Can't allocate %u bytes for %s\n",
> +		sprintf(buf, "Can't allocate %lu bytes for %s\n",
> 			size, description);
> 		fatal_error(ctx, buf);
> 	}
> --
> 2.21.3
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

Re: [PATCH 3/4] e2fsck: use the right conversion specifier in e2fsck_allocate_memory()

[Theodore Y. Ts'o]

Thanks, I had fixed this in one place, but the fix under the #ifdef
DEBUG_ALLOCATE_MEMORY needed, so that part has been applied.

		      	      	 - Ted
				 



On Fri, Jun 05, 2020 at 10:14:41AM +0200, Lukas Czerner wrote:
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> ---
>  e2fsck/util.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/e2fsck/util.c b/e2fsck/util.c
> index 88e0ea8a..79916928 100644
> --- a/e2fsck/util.c
> +++ b/e2fsck/util.c
> @@ -123,10 +123,10 @@ void *e2fsck_allocate_memory(e2fsck_t ctx, unsigned long size,
>  	char buf[256];
>  
>  #ifdef DEBUG_ALLOCATE_MEMORY
> -	printf("Allocating %u bytes for %s...\n", size, description);
> +	printf("Allocating %lu bytes for %s...\n", size, description);
>  #endif
>  	if (ext2fs_get_memzero(size, &ret)) {
> -		sprintf(buf, "Can't allocate %u bytes for %s\n",
> +		sprintf(buf, "Can't allocate %lu bytes for %s\n",
>  			size, description);
>  		fatal_error(ctx, buf);
>  	}
> -- 
> 2.21.3
> 

[PATCH 4/4] ext2fs: remove unused variable 'left'

[Lukas Czerner]

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 lib/ext2fs/swapfs.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/ext2fs/swapfs.c b/lib/ext2fs/swapfs.c
index 5b93b501..bc9f3230 100644
--- a/lib/ext2fs/swapfs.c
+++ b/lib/ext2fs/swapfs.c
@@ -456,12 +456,11 @@ errcode_t ext2fs_dirent_swab_out2(ext2_filsys fs, char *buf,
 {
 	errcode_t	retval;
 	char		*p, *end;
-	unsigned int	rec_len, left;
+	unsigned int	rec_len;
 	struct ext2_dir_entry *dirent;
 
 	p = buf;
 	end = buf + size;
-	left = size;
 	while (p < end) {
 		dirent = (struct ext2_dir_entry *) p;
 		retval = ext2fs_get_rec_len(fs, dirent, &rec_len);
-- 
2.21.3

Re: [PATCH 4/4] ext2fs: remove unused variable 'left'

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 883 bytes --]

On Jun 5, 2020, at 2:14 AM, Lukas Czerner <lczerner@redhat.com> wrote:
> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>

Reviewed-by: Andreas Dilger <adilger@dilger.ca>

> ---
> lib/ext2fs/swapfs.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/lib/ext2fs/swapfs.c b/lib/ext2fs/swapfs.c
> index 5b93b501..bc9f3230 100644
> --- a/lib/ext2fs/swapfs.c
> +++ b/lib/ext2fs/swapfs.c
> @@ -456,12 +456,11 @@ errcode_t ext2fs_dirent_swab_out2(ext2_filsys fs, char *buf,
> {
> 	errcode_t	retval;
> 	char		*p, *end;
> -	unsigned int	rec_len, left;
> +	unsigned int	rec_len;
> 	struct ext2_dir_entry *dirent;
> 
> 	p = buf;
> 	end = buf + size;
> -	left = size;
> 	while (p < end) {
> 		dirent = (struct ext2_dir_entry *) p;
> 		retval = ext2fs_get_rec_len(fs, dirent, &rec_len);
> --
> 2.21.3
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

Re: [PATCH 4/4] ext2fs: remove unused variable 'left'

[Theodore Y. Ts'o]

On Fri, Jun 05, 2020 at 10:14:42AM +0200, Lukas Czerner wrote:
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>

Thanks, applied.

					- Ted

Re: [PATCH 1/4] e2fsck: remove unused variable 'new_array'

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 772 bytes --]


> On Jun 5, 2020, at 2:14 AM, Lukas Czerner <lczerner@redhat.com> wrote:
> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>

Reviewed-by: Andreas Dilger <adilger@dilger.ca>

> ---
> e2fsck/rehash.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/e2fsck/rehash.c b/e2fsck/rehash.c
> index 1616d07a..b356b92d 100644
> --- a/e2fsck/rehash.c
> +++ b/e2fsck/rehash.c
> @@ -109,7 +109,7 @@ static int fill_dir_block(ext2_filsys fs,
> 			  void *priv_data)
> {
> 	struct fill_dir_struct	*fd = (struct fill_dir_struct *) priv_data;
> -	struct hash_entry 	*new_array, *ent;
> +	struct hash_entry 	*ent;
> 	struct ext2_dir_entry 	*dirent;
> 	char			*dir;
> 	unsigned int		offset, dir_offset, rec_len, name_len;
> --
> 2.21.3
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

Re: [PATCH 1/4] e2fsck: remove unused variable 'new_array'

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 751 bytes --]


> On Jun 5, 2020, at 2:14 AM, Lukas Czerner <lczerner@redhat.com> wrote:
> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> ---
> e2fsck/rehash.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/e2fsck/rehash.c b/e2fsck/rehash.c
> index 1616d07a..b356b92d 100644
> --- a/e2fsck/rehash.c
> +++ b/e2fsck/rehash.c
> @@ -109,7 +109,7 @@ static int fill_dir_block(ext2_filsys fs,
> 			  void *priv_data)
> {
> 	struct fill_dir_struct	*fd = (struct fill_dir_struct *) priv_data;
> -	struct hash_entry 	*new_array, *ent;
> +	struct hash_entry 	*ent;

PS - there is a space before the tab after "hash_entry" that could be removed.
Ted could fix this when the patch is applied.

Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

Re: [PATCH 1/4] e2fsck: remove unused variable 'new_array'

[Lukas Czerner]

On Fri, Jun 05, 2020 at 10:14:39AM +0200, Lukas Czerner wrote:
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> ---
>  e2fsck/rehash.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/e2fsck/rehash.c b/e2fsck/rehash.c
> index 1616d07a..b356b92d 100644
> --- a/e2fsck/rehash.c
> +++ b/e2fsck/rehash.c
> @@ -109,7 +109,7 @@ static int fill_dir_block(ext2_filsys fs,
>  			  void *priv_data)
>  {
>  	struct fill_dir_struct	*fd = (struct fill_dir_struct *) priv_data;
> -	struct hash_entry 	*new_array, *ent;
> +	struct hash_entry 	*ent;
>  	struct ext2_dir_entry 	*dirent;
>  	char			*dir;
>  	unsigned int		offset, dir_offset, rec_len, name_len;
> -- 
> 2.21.3
> 

Ehm, ping ?

-Lukas

Re: [PATCH 1/4] e2fsck: remove unused variable 'new_array'

[Theodore Y. Ts'o]

Thanks, this was fixed already.

				- Ted