On Thu, Oct 7, 2010 at 1:10 AM, Goffredo Baroncelli <kreij...@libero.it> wrote:
> On Wednesday, 06 October, 2010, David Nicol wrote:
>> the ISO 8601 duration support is very loose, but I believe it is
>> accurate for valid
> In the man page and in the help the syntax is reported as:
>
> btrfs filesystem reclaim <path> [<timeout>]
>
> instead it should be
>
> btrfs filesystem reclaim [<path> [<timeout>]]
>
> and it has to be reported that path is optional and if it is omitted the
> current CWD is taken.
D'oh! yes you are right.
> 2) I think that it is more reasonable avoid a "strict" iso 8601 syntax for the
> time. I suggest to use a simple syntax like
>
> 0.xxxx -> subsecond
> s or nothing -> seconds
> m -> minutes
> h -> hour
> d -> day
> M -> month (but make sense to wait up to a month ?)
> [...]
That's what the function provides, aside from differentiating between
M and m instead of using the ISO8601 disambiguation rule (as explained
on wikipedia.) After sleeping on it I'm thinking that a better route
would be either
(1) having the timeout in seconds.subseconds and providing a separate
tool that will parse ISO8601 durations, which is suggested to invoke
in backticks. Greater reusability and fewer library functions linked
into the binaries for the win.
or
(2) only supporting the WDHMS designators, nothing larger, (not big-M
or Y), thus removing the ambiguity. Y or M before [TWDH] would be a
fatal error. No longer caring how many seconds in a year for the win.
> 3) As minor issue I have to point out that "reclaim" seems (to me) that the
> user has to start this command in order to obtain more space, like if this
> command starts a garbage collector. In any case the help/man page explain
> clearly the behavior of the command.
My hope is that the current behavior is a temporary stand-in for
something to be developed later that will more aggressively collect
garbage.
> + switch (*endptr)
> + {
> + case 'P': /* anchor */ case 'p':
> + if (ptr > P)
> + fprintf(stderr, "ignoring non-initial P "
> + "in ISO8601 duration string %s\n", P);
> + break;
> + case 'Y': /* years */ case 'y':
> + component *= 12;
> + BIGM:
> + /* average days in a gregorian month */
> + component *= (365.2425 / 12.0);
> + /* ms in a day */
> + component *= ( 24 * 3600 * 1000 );
> + ms += component;
> + break;
> + case 'T': /* Time (not date) anchor */ case 't':
> + M_min = 1;
> + break;
> + case 'W': /* week */ case 'w':
> + component *= 7;
> + case 'D': /* day */ case 'd':
> + component *= 24 ;
> + case 'H': /* hour */ case 'h':
> + component *= 60;
> + M_min = 1;
> + case 'M': /* month, or minute */ case 'm':
> + if (!M_min++)
> + goto BIGM;
> + component *= 60;
> [...]
> In the function I suggest to avoid if possible a goto in a switch case. I
> think that it is more clear to repeat few lines instead of doing a goto.
Well it isn't a whole Duff's device, but by choosing to use the goto
here the constant of year length in seconds only has to appear once. I
guess I could define a symbol. Stacking the cases like this should cut
down on the number of jumps in the compiled machine code, as each
"break" in a switch is short for a jump to the end. I'd be okay with
testing nearer the top so the jump is forward instead of backwards.
I believe that having a goto in a switch statement with a lot of
fallthroughs in its logic (such as this one) highlights that a C
switch statement makes a jump table rather than being a macro for an
oddly crippled series of else-if statements referring to the same
topic, which is what some providers of switch syntax in certain other
languages like to do with the construct.
review: http://en.wikipedia.org/wiki/Duff's_device
and I'm trying to optimize for size, not speed.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
