Marcus Griep <marcus@xxxxxxxx> writes:
> This is a redux of a prior patch as part of a series on count-objects
> but is now split off and submitted on its own as an RFC for a library
> function to be added to strbuf.
Ok, so I looked at the patch again.
> diff --git a/.gitignore b/.gitignore
> index bbaf9de..251537b 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -147,6 +147,7 @@ test-date
> test-delta
> test-dump-cache-tree
> test-genrandom
> +test-human-read
> test-match-trees
> test-parse-options
> test-path-utils
Is it just me or should the test called "test-human-readable"?
> diff --git a/strbuf.c b/strbuf.c
> index 720737d..d9888fb 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -308,3 +308,95 @@ int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint)
> ...
> +{
> + const int maxscale = 7;
This is unused as far as I can tell.
> + strbuf_addf(sb, "%f", sign * val);
> +
> + if (maxlen) {
> + int signlen = sign == -1 ? 1 : 0;
> + maxlen -= (sb->buf[maxlen-1+signlen] == '.' ? 1 : 0);
> + if (maxlen <= 0) {
> + strbuf_setlen(sb, 0);
> + retval = maxlen - 1;
> + } else {
> + strbuf_setlen(sb, maxlen + signlen);
> + }
> + }
This means you print to the buffer and then _truncate_ down to precision,
doesn't it? Shouldn't you be rounding (possibly up if it is above the
midway point)?
For example, if I hand 1638 to you, you would give 1.599Ki back to me, but
if I give you only 4 digits to work with, you do not want to say 1.59Ki;
instead you would rather say 1.60Ki, right?
You would need to compute the number of digits you would want to see
upfront and format the value using "%.*f" with appropriate precision.
> diff --git a/test-human-read.c b/test-human-read.c
> new file mode 100644
> index 0000000..7890922
> --- /dev/null
> +++ b/test-human-read.c
> @@ -0,0 +1,23 @@
> +#include "builtin.h"
> +#include "strbuf.h"
> +
> +int main(int argc, char **argv) {
> + if (argc != 6) {
> + exit(-1);
> + }
> +
> + struct strbuf sb;
Decl after statement.
> + strbuf_init(&sb, 0);
> +
> + int retval = strbuf_append_human_readable(&sb,
> + atof(argv[1]), atoi(argv[2]), atoi(argv[3]), atoi(argv[4]));
> +
> + int failed = strcmp(sb.buf, argv[5]);
> +
> + fprintf( stderr, failed ? "Failure" : "Success" );
> + fprintf( stderr, ": Act '%s'; Exp '%s'\n", sb.buf, argv[5] );
> + fprintf( stderr, "Return Value: %d\n", retval );
Style.
> +
> + if(failed) return -1;
Style.
> + return retval;
> +}
> --
> 1.6.0.1.451.gc8d31
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
