[Ocfs2-tools-devel] [PATCH 12/22] tunefs rework: Add op_set_slot_count.c

[Tao Ma]

Joel Becker wrote:
> On Fri, Aug 01, 2008 at 10:49:01AM +0800, Tao Ma wrote:
>> Joel Becker wrote:
>>> +static int set_slot_count_parse_option(struct tunefs_operation *op,
>>> +				       char *arg)
>>> +{
>>> +	int rc = 1;
>>> +	char *ptr = NULL;
>>> +	long num_slots;
>>> +
>>> +	if (!arg) {
>>> +		errorf("Number of slots not specified\n");
>>> +		goto out;
>>> +	}
>>> +
>>> +	num_slots = strtol(arg, &ptr, 10);
>>> +	if ((num_slots == LONG_MIN) || (num_slots == LONG_MAX)) {
>>> +		errorf("Number of slots is out of range: %s\n", arg);
>>> +		goto out;
>>> +	}
>>> +	if (*ptr != '\0') {
>>> +		errorf("Invalid number: \"%s\"\n", arg);
>>> +		goto out;
>>> +	}
>>> +	if (num_slots < 1) {
>>> +		errorf("At least one slot required\n");
>>> +		goto out;
>>> +	}
>>> +	if (num_slots > INT_MAX) {
>>> +		errorf("Number of slots is out of range: %s\n", arg);
>>> +		goto out;
>>> +	}
>>> +	/*
>>> +	 * We'll check the maximum number of slots after we've opened
>>> +	 * the filesystem and determined the slot map format.
>>> +	 */
>>> +
>>> +	op->to_private = (void *)num_slots;
>>> +	rc = 0;
>>> +
>>> +out:
>>> +	return rc;
>>> +}
>>> +
>>> +static int set_slot_count_run(struct tunefs_operation *op,
>>> +			      ocfs2_filesys *fs, int flags)
>>> +{
>>> +	errcode_t err;
>>> +	int rc = 0;
>>> +	int num_slots = (int)op->to_private;
> 
>> In set_slot_count_parse_option you define num_slots as "long" and here you 
>> use "int". Actually it will not cause problem sine we can never has such a 
>> big num_slots. But should we use the same type?
> 
> 	This is intentional, because I'm using strtol(3) to parse the
> number.  strtol(3) provides error reporting that atoi(3) does not.  Once
> I have the value as a long, I then check the range of the value against
> negative numbers and INT_MAX, thus ensuring it's a positive int. I can
> then use it later as an int, which is what the slot count code wants.
fair enough. So could you please add some comments for this? I am not 
sure whether others will have the same thought as I.

Regards,
Tao