linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Sterba <dsterba@suse.cz>
To: Qu Wenruo <quwenruo@cn.fujitsu.com>
Cc: linux-btrfs@vger.kernel.org
Subject: Re: [PATCH] btrfs-progs: Improve the parse_size() error message.
Date: Wed, 28 May 2014 19:20:18 +0200	[thread overview]
Message-ID: <20140528172017.GS5346@twin.jikos.cz> (raw)
In-Reply-To: <1400572305-19818-1-git-send-email-quwenruo@cn.fujitsu.com>

On Tue, May 20, 2014 at 03:51:45PM +0800, Qu Wenruo wrote:
> When using parse_size(), even non-numeric value is passed, it will only
> give error message "ERROR: size value is empty", which is quite
> confusing for end users.
> 
> This patch will introduce more meaningful error message for the
> following new cases
> 1) Invalid size string (non-numeric string)
> 2) Minus size value (like "-1K")

That's great. A few things below

>  u64 parse_size(char *s)
>  {
> -	int i;
>  	char c;
>  	u64 mult = 1;
> -
> -	for (i = 0; s && s[i] && isdigit(s[i]); i++) ;
> -	if (!i) {
> -		fprintf(stderr, "ERROR: size value is empty\n");
> -		exit(50);
> -	}
> -
> -	if (s[i]) {
> -		c = tolower(s[i]);
> +	long long int ret = 0;
> +	char *endptr;
> +
> +	if (!s)
> +		goto empty;
> +	ret = strtoll(s, &endptr, 10);

> -	return strtoull(s, NULL, 10) * mult;

Now it does not parse sizes from the range LLONG_MAX to ULLONG_MAX (8EiB
to 16EiB). Though we don't have problems with such sizes in real life,
I'd rather keep strtoull so it matches the advertised maximum filesystem
size.

Negative numbers can be simply identified by leading '-'. The only case
where negative number is allowed is in 'fi resize', but the string is
passed to kernel as-is.

> +empty:
> +	fprintf(stderr, "ERROR: Size value is empty\n");
> +	exit(50);
> +suffix:
> +	fprintf(stderr, "ERROR: Illegal suffix contains character '%c' in wrong position\n",
> +		endptr[1]);
> +	exit(51);
> +descriptor:
> +	fprintf(stderr, "ERROR: Unknown size descriptor '%c'\n", c);
> +	exit(52);
> +invalid:
> +	fprintf(stderr, "ERROR: Size value '%s' is invalid\n", s);
> +	exit(53);
> +minus:
> +	fprintf(stderr, "ERROR: Size value '%s' is less equal than 0\n", s);
> +	exit(54);

Please get rid of the error codes, use exit(1). The messages should be
enough to find out what's wrong.

  reply	other threads:[~2014-05-28 17:20 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-20  7:51 [PATCH] btrfs-progs: Improve the parse_size() error message Qu Wenruo
2014-05-28 17:20 ` David Sterba [this message]
2014-05-29  0:41   ` Qu Wenruo
2014-05-28 21:07 ` Mike Fleetwood
2014-05-29  0:41   ` Qu Wenruo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140528172017.GS5346@twin.jikos.cz \
    --to=dsterba@suse.cz \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=quwenruo@cn.fujitsu.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).