Hi Stefan,
On 2012-10-15 22:36, Stefan Behrens wrote:
Hi Goffredo
On 10/15/2012 21:15, Goffredo Baroncelli wrote:
From: Goffredo Baroncelli <kreijack@xxxxxxxxx>
Move parse_size() in utils.c, because it is used both from
cmds-filesystem.c and mkfs.c.
Makes sense.
(Jan also sent such a patch on 1 Feb 2011
<http://permalink.gmane.org/gmane.comp.file-systems.btrfs/9016>, as
part of the patch for speed profiles and dedicated log devices. But
the whole patch series was not integrated.)
Moreover added check about overflow, support to further units (
t=tera, e=exa, z=zetta, y=yotta).
Don't make it too complicated, please. Btrfs cannot handle more than
2^64 bytes.
yeaa I didn't resisted :-)
Correct a bug which happens if a value like 123MB is passed: before
the function returned 123 instead of 123*1024*1024
Yes, that should be fixed. 'B' is taken as the size multiplier 'byte'
(times one) and 'M' is silently ignored.
[....]
+/*
+ * see http://gurmeet.net/puzzles/fast-bit-counting-routines/ for
+ * further algorithms, I used the only one which I understood :-)
+ */
+static int bitcount(u64 v)
+{
+ int n;
+ for(n=0; v ; n++, v >>= 1) ;
+
+ return n;
+}
IMO, something like bitcount() is not needed, much too complicated for
such a minor issue.
I fear that if somebody pass something like 16E, it got un-predicible
results. Let me to wait a day to see if anyone has a different opinion
on it. If nobody tell otherwise I will remove this check.
+
+u64 parse_size(char *s)
+{
+ int shift = 0;
+ u64 ret;
+ int n, i;
+
+ s = strdup(s);
Either don't call strdup() or free() the memory at the end.
This was a my BUG, I removed the needing of strdup(), but I don't remove
the strdup() itself :-)
Thanks to catch it.
+
+ for( i = 0 ; s[i] && isdigit(s[i]) ; i++ ) ;
+ switch (tolower(s[i])) {
+ /* note: the yobibyte and the zebibyte don't fit
+ in a u64, they need an u128 !!! */
If the comment is correct, please remove yobi, zebi and the comment :)
ok... (but zfs supports zettabyte filesystem.... )
+ case 'y': /* yobibyte */
+ shift += 10;
+ case 'z': /* zebibyte */
+ shift += 10;
+ case 'e': /* exbibyte */
+ shift += 10;
+ case 'p': /* pebibyte */
+ shift += 10;
+ case 't': /* tetibyte */
+ shift += 10;
+ case 'g': /* gibibyte */
+ shift += 10;
+ case 'm': /* mebibyte */
+ shift += 10;
+ case 'k': /* kibibyte */
+ shift += 10;
+ case 0:
This should be "case 'b'" at the end in order to maintain backward
compatibility.
Ok, this make sense
+ break;
+ default:
+ fprintf(stderr, "ERROR: Unknown size descriptor %c\n",
+ s[i]);
+ exit(1);
+ }
+ ret = strtoull(s, 0, 0);
+ n = bitcount(ret);
+
+ if( ( n + shift ) > ( sizeof(u64) * CHAR_BIT )) {
+ fprintf(stderr, "ERROR: Overflow, the value '%s' is too big\n",
+ s);
+ fprintf(stderr, "ERROR: Abort\n");
+ exit(1);
+ }
IMO, bitcount() and the check afterwards should be removed.
+
+ return ret << shift;
+}
+
+
diff --git a/utils.h b/utils.h
index 3a0368b..180b3f9 100644
--- a/utils.h
+++ b/utils.h
@@ -46,4 +46,5 @@ int check_label(char *input);
int get_mountpt(char *dev, char *mntpt, size_t size);
int btrfs_scan_block_devices(int run_ioctl);
+u64 parse_size(char *s);
#endif
I updated the patch on my repo (see first email), if someone want to
make some tests. As explained above I will wait a day, then I will
re-issue the patch.
BR
G.Baroncelli
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
