Re: [PATCH] fs: ramfs: make chunk counting in truncate() better readable

["Marcin Niestrój" <m.niestroj@grinn-global.com>]

I think I was a little bit too early with review :) Below I have some
comments.

Marcin Niestrój <m.niestroj@grinn-global.com> writes:

> Sascha Hauer <s.hauer@pengutronix.de> writes:
>
>> In ramfs_truncate() "newchunks" denotes the number of chunks we
>> want to have after the call. We decrease that number while iterating
>> over the existing chunks and decrease it further with every newly
>> allocated chunk until "newchunks" is zero.
>> This is a bit hard to read. Instead we drop the decreasing while
>> iterating over existing chunks and increase "oldchunks" while allocating
>> until it reaches "newchunks".
>>
>> This is mainly done to make the next patch easier.
>>
>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>> ---
>>  fs/ramfs.c | 9 ++++-----
>>  1 file changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/ramfs.c b/fs/ramfs.c
>> index 09dafe02ae..8ba8d77de9 100644
>> --- a/fs/ramfs.c
>> +++ b/fs/ramfs.c
>> @@ -384,19 +384,18 @@ static int ramfs_truncate(struct device_d *dev, FILE *f, ulong size)
>>  			if (!node->data)
>>  				return -ENOMEM;
>>  			data = node->data;
>> +			newchunks = 1;

What is the reason of this instruction? What if 'size' == 16384 and we
do it on freshly opened file (with truncate(fd, 16384)? 'newchunk'
should be 2 in that case, or not?

Regards,
Marcin

>>  		}
>>  
>> -		newchunks--;
>> -		while (data->next) {
>> -			newchunks--;
>> +		while (data->next)
>>  			data = data->next;
>> -		}
>>  
>> -		while (newchunks--) {
>> +		while (newchunks > oldchunks) {
>>  			data->next = ramfs_get_chunk();
>>  			if (!data->next)
>>  				return -ENOMEM;
>>  			data = data->next;
>> +			oldchunks++;
>>  		}
>>  	}
>>  	node->size = size;
>
> Reviewed-by: Marcin Niestroj <m.niestroj@grinn-global.com>

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox