[U-Boot,1/1] fs: fat: avoid useless conversion when calling getcluster

Message ID 20180213181858.10063-1-xypron.glpk@gmx.de
State Superseded, archived
Delegated to: Tom Rini
Headers show
Series
  • [U-Boot,1/1] fs: fat: avoid useless conversion when calling getcluster
Related show

Commit Message

Heinrich Schuchardt Feb. 13, 2018, 6:18 p.m.
Parameter size of function get_cluster() is of type unsigned long. It makes
no sense to convert actsize to int before passing it to get_cluster as
size.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 fs/fat/fat.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Lukasz Majewski Feb. 13, 2018, 9:27 p.m. | #1
On Tue, 13 Feb 2018 19:18:58 +0100
Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

> Parameter size of function get_cluster() is of type unsigned long. It
> makes no sense to convert actsize to int before passing it to
> get_cluster as size.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  fs/fat/fat.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> index dd7888cd6d4..a3c7bf604eb 100644
> --- a/fs/fat/fat.c
> +++ b/fs/fat/fat.c
> @@ -353,7 +353,7 @@ static int get_contents(fsdata *mydata, dir_entry
> *dentptr, loff_t pos, if (pos) {
>  		actsize = min(filesize, (loff_t)bytesperclust);
>  		if (get_cluster(mydata, curclust,
> get_contents_vfatname_block,
> -				(int)actsize) != 0) {
> +				(unsigned long)actsize) != 0) {
>  			printf("Error reading cluster\n");
>  			return -1;
>  		}
> @@ -393,14 +393,16 @@ static int get_contents(fsdata *mydata,
> dir_entry *dentptr, loff_t pos, 
>  		/* get remaining bytes */
>  		actsize = filesize;
> -		if (get_cluster(mydata, curclust, buffer,
> (int)actsize) != 0) {
> +		if (get_cluster(mydata, curclust, buffer,
> +				(unsigned long)actsize) != 0) {
>  			printf("Error reading cluster\n");
>  			return -1;
>  		}
>  		*gotsize += actsize;
>  		return 0;
>  getit:
> -		if (get_cluster(mydata, curclust, buffer,
> (int)actsize) != 0) {
> +		if (get_cluster(mydata, curclust, buffer,
> +				(unsigned long)actsize) != 0) {
>  			printf("Error reading cluster\n");
>  			return -1;
>  		}

Reviewed-by: Lukasz Majewski <lukma@denx.de>


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
Tuomas Tynkkynen Feb. 13, 2018, 9:50 p.m. | #2
Hi,

On Tue, 13 Feb 2018 19:18:58 +0100
Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

> Parameter size of function get_cluster() is of type unsigned long. It makes
> no sense to convert actsize to int before passing it to get_cluster as
> size.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  fs/fat/fat.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
> index dd7888cd6d4..a3c7bf604eb 100644
> --- a/fs/fat/fat.c
> +++ b/fs/fat/fat.c
> @@ -353,7 +353,7 @@ static int get_contents(fsdata *mydata, dir_entry *dentptr, loff_t pos,
>  	if (pos) {
>  		actsize = min(filesize, (loff_t)bytesperclust);
>  		if (get_cluster(mydata, curclust, get_contents_vfatname_block,
> -				(int)actsize) != 0) {
> +				(unsigned long)actsize) != 0) {
>  			printf("Error reading cluster\n");

If the type is unsigned long, the explicit cast here seems redundant.
Heinrich Schuchardt Feb. 14, 2018, 7:17 a.m. | #3
On 02/13/2018 10:50 PM, Tuomas Tynkkynen wrote:
> Hi,
> 
> On Tue, 13 Feb 2018 19:18:58 +0100
> Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> 
>> Parameter size of function get_cluster() is of type unsigned long. It makes
>> no sense to convert actsize to int before passing it to get_cluster as
>> size.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>>   fs/fat/fat.c | 8 +++++---
>>   1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/fat/fat.c b/fs/fat/fat.c
>> index dd7888cd6d4..a3c7bf604eb 100644
>> --- a/fs/fat/fat.c
>> +++ b/fs/fat/fat.c
>> @@ -353,7 +353,7 @@ static int get_contents(fsdata *mydata, dir_entry *dentptr, loff_t pos,
>>   	if (pos) {
>>   		actsize = min(filesize, (loff_t)bytesperclust);
>>   		if (get_cluster(mydata, curclust, get_contents_vfatname_block,
>> -				(int)actsize) != 0) {
>> +				(unsigned long)actsize) != 0) {
>>   			printf("Error reading cluster\n");
> 
> If the type is unsigned long, the explicit cast here seems redundant.
> 
actsize is of type l_off_t. As long as loff_t does not have more bits 
than unsigned long you are right. But the C language does not require 
this to hold true.

But why shouldn't we use the same type loff_t in the interface of 
get_cluster. I will update the patch.

Best regards

Heinrich

Patch

diff --git a/fs/fat/fat.c b/fs/fat/fat.c
index dd7888cd6d4..a3c7bf604eb 100644
--- a/fs/fat/fat.c
+++ b/fs/fat/fat.c
@@ -353,7 +353,7 @@  static int get_contents(fsdata *mydata, dir_entry *dentptr, loff_t pos,
 	if (pos) {
 		actsize = min(filesize, (loff_t)bytesperclust);
 		if (get_cluster(mydata, curclust, get_contents_vfatname_block,
-				(int)actsize) != 0) {
+				(unsigned long)actsize) != 0) {
 			printf("Error reading cluster\n");
 			return -1;
 		}
@@ -393,14 +393,16 @@  static int get_contents(fsdata *mydata, dir_entry *dentptr, loff_t pos,
 
 		/* get remaining bytes */
 		actsize = filesize;
-		if (get_cluster(mydata, curclust, buffer, (int)actsize) != 0) {
+		if (get_cluster(mydata, curclust, buffer,
+				(unsigned long)actsize) != 0) {
 			printf("Error reading cluster\n");
 			return -1;
 		}
 		*gotsize += actsize;
 		return 0;
 getit:
-		if (get_cluster(mydata, curclust, buffer, (int)actsize) != 0) {
+		if (get_cluster(mydata, curclust, buffer,
+				(unsigned long)actsize) != 0) {
 			printf("Error reading cluster\n");
 			return -1;
 		}