All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Sandeen <sandeen@redhat.com>
To: Zach Brown <zab@redhat.com>
Cc: linux-btrfs@vger.kernel.org
Subject: Re: [PATCH 01/17] btrfs-progs: Unify size-parsing
Date: Mon, 25 Feb 2013 17:37:05 -0600	[thread overview]
Message-ID: <512BF5A1.8040404@redhat.com> (raw)
In-Reply-To: <20130225232651.GE1658@lenny.home.zabbo.net>

On 2/25/13 5:26 PM, Zach Brown wrote:
>>  		case 'e':
>>  			mult *= 1024;
>> +			/* Fallthrough */
> 
> These comments still annoy me :).  

it shuts up coverity & other static checkers which are worried about
a missing break...

> And really, that code kind of annoys
> me too.  That's a lot of duplicated code for a mapping of characters to
> powers of 1024.
> 
> How about.. 
> 
> u64 pow_u64(u64 x, unsigned y)
> {
> 	u64 ret = 1;
> 
> 	while (y--)
> 		ret *= x;
> 
> 	return ret;
> }
> 
> u64 get_mult(char unit)
> {
> 	static char *units = "bkmgtpe";
> 	char *found = index(units, unit);
> 
> 	if (found)
> 		return pow_u64(1024, found - units);
> 	return 0;
> }
> 
> Seems like a lot less noise for the same functionality.

If you want to submit that patch as a further cleanup I'd review it ;)

But TBH (going out on a limb crossing Zach here ;) my feeble brain
is easily confused by your clever code.  I'd rather just do the
simple thing simply...

-Eric

> - z
> 


  reply	other threads:[~2013-02-25 23:37 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-25 22:54 [PATCH 00/17] btrfs-progs: More misc fixes & cleanups Eric Sandeen
2013-02-25 22:54 ` [PATCH 01/17] btrfs-progs: Unify size-parsing Eric Sandeen
2013-02-25 23:26   ` Zach Brown
2013-02-25 23:37     ` Eric Sandeen [this message]
2013-02-26  0:26       ` Zach Brown
2013-02-26 18:50   ` Goffredo Baroncelli
2013-02-26 20:17     ` Eric Sandeen
2013-02-26 21:15       ` Goffredo Baroncelli
2013-02-25 22:54 ` [PATCH 02/17] btrfs-progs: fix btrfs_get_subvol cut/paste error Eric Sandeen
2013-02-25 22:54 ` [PATCH 03/17] btrfs-progs: Remove write-only var fdres in cmd_dev_stats() Eric Sandeen
2013-02-25 22:54 ` [PATCH 04/17] btrfs-progs: btrfs_list_get_path_rootid error handling Eric Sandeen
2013-02-25 22:54 ` [PATCH 05/17] btrfs-progs: avoid double-free in __btrfs_map_block Eric Sandeen
2013-02-25 22:54 ` [PATCH 06/17] btrfs-progs: fix open error test in cmd_start_replace Eric Sandeen
2013-02-25 22:54 ` [PATCH 07/17] btrfs-progs: fix close of error fd in scrub cancel Eric Sandeen
2013-02-25 22:54 ` [PATCH 08/17] btrfs-progs: more scrub cancel error handling Eric Sandeen
2013-02-25 22:54 ` [PATCH 09/17] btrfs-progs: free memory before error exit in read_whole_eb Eric Sandeen
2013-02-25 22:54 ` [PATCH 10/17] btrfs-progs: don't call close on error fd Eric Sandeen
2013-02-25 22:54 ` [PATCH 11/17] btrfs-progs: provide positive errno to strerror in cmd_restore Eric Sandeen
2013-02-25 22:54 ` [PATCH 12/17] btrfs-progs: free allocated di_args in cmd_start_replace Eric Sandeen
2013-02-25 22:54 ` [PATCH 13/17] btrfs-progs: close fd on cmd_subvol_get_default return Eric Sandeen
2013-02-25 22:54 ` [PATCH 14/17] btrfs-progs: fix mem leak in resolve_root Eric Sandeen
2013-02-26  0:36   ` Shilong Wang
2013-02-26  4:36     ` Eric Sandeen
2013-02-27 13:03       ` David Sterba
2013-02-27 13:12         ` Shilong Wang
2013-02-25 22:54 ` [PATCH 15/17] btrfs-progs: Tidy up resolve_root Eric Sandeen
2013-02-25 22:54 ` [PATCH 16/17] btrfs-progs: fix fd leak in cmd_subvol_set_default Eric Sandeen
2013-02-26 18:46   ` Goffredo Baroncelli
2013-02-26 20:10     ` Eric Sandeen
2013-02-26 21:04       ` Goffredo Baroncelli
2013-02-27 12:38         ` David Sterba
2013-02-25 22:54 ` [PATCH 17/17] btrfs-progs: replace strtok_r with strsep Eric Sandeen
2013-02-26 18:47   ` Goffredo Baroncelli
2013-02-26 20:13     ` Eric Sandeen
2013-02-26 20:20     ` [PATCH 17/17 V2] " Eric Sandeen
2013-02-26 20:40       ` Ilya Dryomov
2013-02-26 20:46         ` Eric Sandeen
2013-02-26 21:07           ` Ilya Dryomov
2013-02-26 21:50             ` [PATCH 17/17 V3] btrfs-progs: initialize save_ptr prior to strtok_r Eric Sandeen
2013-02-27 13:54 ` [PATCH 00/17] btrfs-progs: More misc fixes & cleanups David Sterba

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=512BF5A1.8040404@redhat.com \
    --to=sandeen@redhat.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=zab@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.