Re: [PATCH 3/7] mkfs: remove intermediate getstr followed by getnum

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 7/20/17 10:54 AM, Darrick J. Wong wrote:
> On Thu, Jul 20, 2017 at 11:29:28AM +0200, Jan Tulak wrote:
>> Some options loaded a number as a string with getstr and converted it to
>> number with getnum later in the code, without any reason for this
>> approach.  (They were probably forgotten in some past cleaning.)
>>
>> This patch changes them to skip the string and use getnum directly in
>> the main option-parsing loop, as do all the other numerical options.
>> And as we now have two variables of the same type for the same value,
>> merge them together. (e.g. former string dsize and number dbytes).
>>
>> Signed-off-by: Jan Tulak <jtulak@xxxxxxxxxx>
>> Reviewed-by: Eric Sandeen <sandeen@xxxxxxxxxx>
>> Reviewed-by: Luis R. Rodriguez <mcgrof@xxxxxxxxxx>
>>
>> ---
>> In reply to Eric's comment, so it doesn't pop up again:
>> This patch has to be applied after "mkfs: Save raw user input ...",
>> because otherwise we would temporary lose the access to strings
>> with user-given values and thus wouldn't be able to report them in
>> case of any issue. 
>> ---
>>  mkfs/xfs_mkfs.c | 90 ++++++++++++++++++++++++++-------------------------------
>>  1 file changed, 41 insertions(+), 49 deletions(-)
>>
>> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
>> index f2f6468e..127f14c3 100644
>> --- a/mkfs/xfs_mkfs.c
>> +++ b/mkfs/xfs_mkfs.c
>> @@ -1345,6 +1345,7 @@ getnum(
>>  	long long		c;
>>  
>>  	check_opt(opts, index, false);
>> +	set_conf_raw(opts, index, str);
>>  	/* empty strings might just return a default value */
>>  	if (!str || *str == '\0') {
>>  		if (sp->flagval == SUBOPT_NEEDS_VAL)
>> @@ -1432,12 +1433,12 @@ main(
>>  	char			*dfile;
>>  	int			dirblocklog;
>>  	int			dirblocksize;
>> -	char			*dsize;
>> +	int			dbytes;

<snip>


>> @@ -2179,10 +2180,7 @@ _("rmapbt not supported with realtime devices\n"));
>>  	}
>>  
>>  
>> -	if (dsize) {
>> -		__uint64_t dbytes;
>> -
>> -		dbytes = getnum(dsize, &dopts, D_SIZE);
>> +	if (dbytes) {
> 
> Why has dbytes been demoted from uint64_t to int?  This eliminates the
> ability to -d size=8G, right?

That wasn't a problem in the version I reviewed.... pls don't keep reviewed-by's
on patches that change from the reviewed version.  Further, best to indicate
explicitly what has changed since the last version, under the "---"

Thanks,
-Eric

--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux