FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > ArchLinux > ArchLinux Pacman Development

 
 
LinkBack Thread Tools
 
Old 02-25-2011, 01:51 PM
Dan McGee
 
Default Add utility functions to convert sizes to strings

On Mon, Feb 21, 2011 at 1:02 PM, Jakob Gruber <jakob.gruber@gmail.com> wrote:
> char *size_to_human_string(off_t bytes);
> char *size_to_human_string_short(off_t bytes);
>
> Scale to the first unit with which the value will be less or equal to
> 2048. size_to_human_string_short uses short unit labels ("K","M",...)
> instead of the default ("KB", "MB"), uses only one decimal digit and has
> no space between the value and the label.
>
> For example, size_to_human_string: "1.31 MB", short: "1.3M".
>
> char *size_to_human_string_mb(off_t bytes);
> char *size_to_human_string_kb(off_t bytes);
>
> Convert to MB and KB respectively and format identically to
> size_to_human_string.
>
> Signed-off-by: Jakob Gruber <jakob.gruber@gmail.com>
> ---
> *src/pacman/callback.c | * 46 ++++++------------------
> *src/pacman/package.c *| * 20 ++++++-----
> *src/pacman/query.c * *| * *7 ++--
> *src/pacman/sync.c * * | * *7 ++--
> *src/pacman/util.c * * | * 94 +++++++++++++++++++++++++++++++++++++++++--------
> *src/pacman/util.h * * | * *4 ++
> *6 files changed, 111 insertions(+), 67 deletions(-)
>
> diff --git a/src/pacman/callback.c b/src/pacman/callback.c
> index f1e71bb..8ec4c3e 100644
> --- a/src/pacman/callback.c
> +++ b/src/pacman/callback.c
> @@ -484,17 +484,16 @@ void cb_dl_progress(const char *filename, off_t file_xfered, off_t file_total)
> *{
> * * * *int infolen;
> * * * *int filenamelen;
> - * * * char *fname, *p;
> + * * * char *fname, *p, *ratestr, *xferedstr;
> * * * */* used for wide character width determination and printing */
> * * * *int len, wclen, wcwid, padwid;
> * * * *wchar_t *wcfname;
>
> * * * *int totaldownload = 0;
> * * * *off_t xfered, total;
> - * * * double rate = 0.0, timediff = 0.0, f_xfered = 0.0;
> + * * * double rate = 0.0, timediff = 0.0;
> * * * *unsigned int eta_h = 0, eta_m = 0, eta_s = 0;
> * * * *int file_percent = 0, total_percent = 0;
> - * * * char rate_size = 'K', xfered_size = 'K';
>
> * * * *if(config->noprogressbar || file_total == -1) {
> * * * * * * * *if(file_xfered == 0) {
> @@ -556,7 +555,7 @@ void cb_dl_progress(const char *filename, off_t file_xfered, off_t file_total)
> * * * * * * * *diff_sec = current_time.tv_sec - initial_time.tv_sec;
> * * * * * * * *diff_usec = current_time.tv_usec - initial_time.tv_usec;
> * * * * * * * *timediff = diff_sec + (diff_usec / 1000000.0);
> - * * * * * * * rate = xfered / (timediff * 1024.0);
> + * * * * * * * rate = xfered / timediff;
>
> * * * * * * * */* round elapsed time to the nearest second */
> * * * * * * * *eta_s = (int)(timediff + 0.5);
> @@ -568,10 +567,10 @@ void cb_dl_progress(const char *filename, off_t file_xfered, off_t file_total)
> * * * * * * * * * * * */* return if the calling interval was too short */
> * * * * * * * * * * * *return;
> * * * * * * * *}
> - * * * * * * * rate = (xfered - xfered_last) / (timediff * 1024.0);
> + * * * * * * * rate = (xfered - xfered_last) / timediff;
> * * * * * * * */* average rate to reduce jumpiness */
> * * * * * * * *rate = (rate + 2 * rate_last) / 3;
> - * * * * * * * eta_s = (total - xfered) / (rate * 1024.0);
> + * * * * * * * eta_s = (total - xfered) / (rate * 1024.0 * 1024.0);
> * * * * * * * *rate_last = rate;
> * * * * * * * *xfered_last = xfered;
> * * * *}
> @@ -625,38 +624,15 @@ void cb_dl_progress(const char *filename, off_t file_xfered, off_t file_total)
>
> * * * *}
>
> - * * * /* Awesome formatting for progress bar. *We need a mess of Kb->Mb->Gb stuff
> - * * * ** here. We'll use limit of 2048 for each until we get some empirical */
> - * * * /* rate_size = 'K'; was set above */
> - * * * if(rate > 2048.0) {
> - * * * * * * * rate /= 1024.0;
> - * * * * * * * rate_size = 'M';
> - * * * * * * * if(rate > 2048.0) {
> - * * * * * * * * * * * rate /= 1024.0;
> - * * * * * * * * * * * rate_size = 'G';
> - * * * * * * * * * * * /* we should not go higher than this for a few years (9999.9 Gb/s?)*/
> - * * * * * * * }
> - * * * }
> -
> - * * * f_xfered = xfered / 1024.0; /* convert to K by default */
> - * * * /* xfered_size = 'K'; was set above */
> - * * * if(f_xfered > 2048.0) {
> - * * * * * * * f_xfered /= 1024.0;
> - * * * * * * * xfered_size = 'M';
> - * * * * * * * if(f_xfered > 2048.0) {
> - * * * * * * * * * * * f_xfered /= 1024.0;
> - * * * * * * * * * * * xfered_size = 'G';
> - * * * * * * * * * * * /* I should seriously hope that archlinux packages never break
> - * * * * * * * * * * * ** the 9999.9GB mark... we'd have more serious problems than the progress
> - * * * * * * * * * * * ** bar in pacman */
> - * * * * * * * }
> - * * * }
> + * * * ratestr = size_to_human_string_short((off_t)rate);
> + * * * xferedstr = size_to_human_string_short(xfered);
>
> * * * */* 1 space + filenamelen + 1 space + 7 for size + 1 + 7 for rate + 2 for /s + 1 space + 8 for eta */
> - * * * printf(" %ls%-*s %6.1f%c %#6.1f%c/s %02u:%02u:%02u", wcfname,
> - * * * * * * * * * * * padwid, "", f_xfered, xfered_size,
> - * * * * * * * * * * * rate, rate_size, eta_h, eta_m, eta_s);
> + * * * printf(" %ls%-*s %7s %7s/s %02u:%02u:%02u", wcfname,
> + * * * * * * * * * * * padwid, "", xferedstr, ratestr, eta_h, eta_m, eta_s);
>
> + * * * free(ratestr);
> + * * * free(xferedstr);
> * * * *free(fname);
> * * * *free(wcfname);
>
> diff --git a/src/pacman/package.c b/src/pacman/package.c
> index 77a5ee7..156d257 100644
> --- a/src/pacman/package.c
> +++ b/src/pacman/package.c
> @@ -50,7 +50,7 @@ void dump_pkg_full(pmpkg_t *pkg, int level)
> *{
> * * * *const char *reason;
> * * * *time_t bdate, idate;
> - * * * char bdatestr[50] = "", idatestr[50] = "";
> + * * * char bdatestr[50] = "", idatestr[50] = "", *size;
I'd prefer these defined on different lines; yes they are the "same"
type but it can be confusing.
> * * * *const alpm_list_t *i;
> * * * *alpm_list_t *requiredby = NULL, *depstrings = NULL;
>
> @@ -105,17 +105,19 @@ void dump_pkg_full(pmpkg_t *pkg, int level)
> * * * *}
> * * * *list_display(_("Conflicts With :"), alpm_pkg_get_conflicts(pkg));
> * * * *list_display(_("Replaces * * * :"), alpm_pkg_get_replaces(pkg));
> +
> + * * * size = size_to_human_string_kb(alpm_pkg_get_size(pkg));
> * * * *if(level < 0) {
> - * * * * * * * printf(_("Download Size *: %6.2f K
"),
> - * * * * * * * * * * * (double)alpm_pkg_get_size(pkg) / 1024.0);
> - * * * }
> - * * * if(level == 0) {
> - * * * * * * * printf(_("Compressed Size: %6.2f K
"),
> - * * * * * * * * * * * (double)alpm_pkg_get_size(pkg) / 1024.0);
> + * * * * * * * printf(_("Download Size *: %s
"), size);
> + * * * } else if(level == 0) {
> + * * * * * * * printf(_("Compressed Size: %s
"), size);
> * * * *}
> + * * * free(size);
> +
> + * * * size = size_to_human_string_kb(alpm_pkg_get_isize(pkg));
> + * * * printf(_("Installed Size : %s
"), size);
> + * * * free(size);
>
> - * * * printf(_("Installed Size : %6.2f K
"),
> - * * * * * * * * * * * (double)alpm_pkg_get_isize(pkg) / 1024.0);
> * * * *string_display(_("Packager * * * :"), alpm_pkg_get_packager(pkg));
> * * * *string_display(_("Architecture * :"), alpm_pkg_get_arch(pkg));
> * * * *string_display(_("Build Date * * :"), bdatestr);
> diff --git a/src/pacman/query.c b/src/pacman/query.c
> index 734875b..8a66927 100644
> --- a/src/pacman/query.c
> +++ b/src/pacman/query.c
> @@ -247,10 +247,9 @@ static int query_search(alpm_list_t *targets)
>
> * * * * * * * */* print the package size with the output if ShowSize option set */
> * * * * * * * *if(!config->quiet && config->showsize) {
> - * * * * * * * * * * * /* Convert byte size to MB */
> - * * * * * * * * * * * double mbsize = (double)alpm_pkg_get_size(pkg) / (1024.0 * 1024.0);
> -
> - * * * * * * * * * * * printf(" [%.2f MB]", mbsize);
> + * * * * * * * * * * * char *size = size_to_human_string_mb(alpm_pkg_get_size(pkg));
> + * * * * * * * * * * * printf(" [%s]", size);
> + * * * * * * * * * * * free(size);
> * * * * * * * *}
>
>
> diff --git a/src/pacman/sync.c b/src/pacman/sync.c
> index 7af1667..21fe4ba 100644
> --- a/src/pacman/sync.c
> +++ b/src/pacman/sync.c
> @@ -349,10 +349,9 @@ static int sync_search(alpm_list_t *syncs, alpm_list_t *targets)
>
> * * * * * * * * * * * */* print the package size with the output if ShowSize option set */
> * * * * * * * * * * * *if(!config->quiet && config->showsize) {
> - * * * * * * * * * * * * * * * /* Convert byte size to MB */
> - * * * * * * * * * * * * * * * double mbsize = (double)alpm_pkg_get_size(pkg) / (1024.0 * 1024.0);
> -
> - * * * * * * * * * * * * * * * printf(" [%.2f MB]", mbsize);
> + * * * * * * * * * * * * * * * char *size = size_to_human_string_mb(alpm_pkg_get_size(pkg));
> + * * * * * * * * * * * * * * * printf(" [%s]", size);
> + * * * * * * * * * * * * * * * free(size);
> * * * * * * * * * * * *}
>
> * * * * * * * * * * * *if (!config->quiet) {
> diff --git a/src/pacman/util.c b/src/pacman/util.c
> index c08ebb1..b651478 100644
> --- a/src/pacman/util.c
> +++ b/src/pacman/util.c
> @@ -501,10 +501,9 @@ void list_display_linebreak(const char *title, const alpm_list_t *list)
> */* prepare a list of pkgs to display */
> *void display_targets(const alpm_list_t *pkgs, int install)
> *{
> - * * * char *str;
> + * * * char *str, *size;
> * * * *const alpm_list_t *i;
> * * * *off_t isize = 0, dlsize = 0;
> - * * * double mbisize = 0.0, mbdlsize = 0.0;
> * * * *alpm_list_t *targets = NULL;
>
> * * * *if(!pkgs) {
> @@ -522,10 +521,10 @@ void display_targets(const alpm_list_t *pkgs, int install)
>
> * * * * * * * */* print the package size with the output if ShowSize option set */
> * * * * * * * *if(config->showsize) {
> - * * * * * * * * * * * double mbsize = (double)alpm_pkg_get_size(pkg) / (1024.0 * 1024.0);
> -
> - * * * * * * * * * * * pm_asprintf(&str, "%s-%s [%.2f MB]", alpm_pkg_get_name(pkg),
> - * * * * * * * * * * * * * * * * * * * alpm_pkg_get_version(pkg), mbsize);
> + * * * * * * * * * * * size = size_to_human_string_mb(alpm_pkg_get_size(pkg));
> + * * * * * * * * * * * pm_asprintf(&str, "%s-%s [%s]", alpm_pkg_get_name(pkg),
> + * * * * * * * * * * * * * * * * * * * alpm_pkg_get_version(pkg), size);
> + * * * * * * * * * * * free(size);
> * * * * * * * *} else {
> * * * * * * * * * * * *pm_asprintf(&str, "%s-%s", alpm_pkg_get_name(pkg),
> * * * * * * * * * * * * * * * * * * * *alpm_pkg_get_version(pkg));
> @@ -533,19 +532,19 @@ void display_targets(const alpm_list_t *pkgs, int install)
> * * * * * * * *targets = alpm_list_add(targets, str);
> * * * *}
>
> - * * * /* Convert byte sizes to MB */
> - * * * mbdlsize = (double)dlsize / (1024.0 * 1024.0);
> - * * * mbisize = (double)isize / (1024.0 * 1024.0);
> -
> * * * *if(install) {
> * * * * * * * *pm_asprintf(&str, _("Targets (%d):"), alpm_list_count(targets));
> * * * * * * * *list_display(str, targets);
> * * * * * * * *free(str);
> * * * * * * * *printf("
");
>
> - * * * * * * * printf(_("Total Download Size: * *%.2f MB
"), mbdlsize);
> + * * * * * * * size = size_to_human_string_mb(dlsize);
> + * * * * * * * printf(_("Total Download Size: * *%s
"), size);
> + * * * * * * * free(size);
> * * * * * * * *if(!(config->flags & PM_TRANS_FLAG_DOWNLOADONLY)) {
> - * * * * * * * * * * * printf(_("Total Installed Size: * %.2f MB
"), mbisize);
> + * * * * * * * * * * * size = size_to_human_string_mb(isize);
> + * * * * * * * * * * * printf(_("Total Installed Size: * %s
"), size);
> + * * * * * * * * * * * free(size);
> * * * * * * * *}
> * * * *} else {
> * * * * * * * *pm_asprintf(&str, _("Remove (%d):"), alpm_list_count(targets));
> @@ -553,7 +552,9 @@ void display_targets(const alpm_list_t *pkgs, int install)
> * * * * * * * *free(str);
> * * * * * * * *printf("
");
>
> - * * * * * * * printf(_("Total Removed Size: * %.2f MB
"), mbisize);
> + * * * * * * * size = size_to_human_string_mb(isize);
> + * * * * * * * printf(_("Total Removed Size: * %s
"), size);
> + * * * * * * * free(size);
I see no reason we should lock any of these to MB units- just use the
general formatter here.

> * * * *}
>
> * * * *FREELIST(targets);
> @@ -594,6 +595,70 @@ static char *pkg_get_location(pmpkg_t *pkg)
> * * * *}
> *}
>
> +/** Converts sizes in bytes into strings.
> + *
> + * @param bytes the size in bytes
> + * @param format the format string to use. The first argument is of type
> + * * * * * * * * * * * * * * * float and represents the size. The second argument is of type
> + * * * * * * * * * * * * * * * char* and represents the unit label. for example, "%.2f %s"
> + * * * * * * * * * * * * * * * could result in "1.23 MB"
> + * @param unit the target unit. can be either one of the short unit
> + * * * * * * * * * * * * * * * labels ("B", "K", ...); or NULL, in which case the first
> + * * * * * * * * * * * * * * * unit which will bring the value to below a threshold of 2048
> + * * * * * * * * * * * * * * * will be chosen.
> + * @param long_units whether to use short ("K") or long ("KB") unit labels
> + *
> + * @return the size string
> + */
> +static char *size_to_human_string_format(off_t bytes, const char *format,
> + * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * const char *unit, int long_labels)
> +{
> + * * * char *ret;
> + * * * static const char *shortlabels[] = {"B", "K", "M", "G", "T", "P"};
> + * * * static const char *longlabels[] = {"B", "KB", "MB", "GB", "TB", "PB"};
> + * * * const char **labels = long_labels ? longlabels : shortlabels;
> + * * * const int unitcount = sizeof(shortlabels) / sizeof(shortlabels[0]);
> + * * * int index;
> + * * * float val = (float)bytes;
> +
> + * * * /* stop conditions:
> + * * * ** 1) a target unit is specified and we are at target unit,
> + * * * ** 2) target unit is NOT specified and value is leq 2048,
> + * * * ** and 3) we are already at the largest unit.s
> + * * * ** note that if an invalid target is specified in 1),
> + * * * ** we convert to the largest unit */
> + * * * for(index = 0; index < unitcount - 1; index++) {
> + * * * * * * * if(unit != NULL && strcmp(shortlabels[index], unit) == 0) break;
I'd make the function param a char (not a "string") to ensure people
aren't doing stupid things like passing a long label in by mistake.
Then you can just use == here.

> + * * * * * * * else if(unit == NULL && val <= 2048.0) break;
> +
> + * * * * * * * val /= 1024.0;
> + * * * }
> +
> + * * * pm_asprintf(&ret, format, val, labels[index]);
> +
> + * * * return ret;
> +}
> +
> +char *size_to_human_string(off_t bytes)
> +{
> + * * * return size_to_human_string_format(bytes, "%.2f %s", NULL, 1);
> +}
> +
> +char *size_to_human_string_mb(off_t bytes)
> +{
> + * * * return size_to_human_string_format(bytes, "%.2f %s", "M", 1);
> +}
> +
> +char *size_to_human_string_kb(off_t bytes)
> +{
> + * * * return size_to_human_string_format(bytes, "%.2f %s", "K", 1);
> +}
> +
> +char *size_to_human_string_short(off_t bytes)
> +{
> + * * * return size_to_human_string_format(bytes, "%.1f%s", NULL, 0);
> +}
> +

I think this API has been over-engineered with all these one-liners.
We aren't exposing this as a library function or anything, so people
can deal with a little complexity in the call themselves. I'd have one
function for all of this, and we can then even skip the malloc/free
cycle:
double value humanize_size(off_t bytes, char max_unit, int
long_labels, const char **label)

Logic stays as is in your format function, expect we don't actually
print the string- the value is returned and label, if provided, gets a
pointer to the correct string. Does this make sense? I'm open for
feedback of course, but I see this as 1) less boilerplate methods in
util.c 2) no free() necessary by caller.

> *void print_packages(const alpm_list_t *packages)
> *{
> * * * *const alpm_list_t *i;
> @@ -638,8 +703,7 @@ void print_packages(const alpm_list_t *packages)
> * * * * * * * */* %s : size */
> * * * * * * * *if(strstr(temp,"%s")) {
> * * * * * * * * * * * *char *size;
> - * * * * * * * * * * * double mbsize = (double)pkg_get_size(pkg) / (1024.0 * 1024.0);
> - * * * * * * * * * * * pm_asprintf(&size, "%.2f", mbsize);
> + * * * * * * * * * * * size = size_to_human_string_format(pkg_get_size(pkg), "%.2f", "M", 0);
> * * * * * * * * * * * *string = strreplace(temp, "%s", size);
> * * * * * * * * * * * *free(size);
> * * * * * * * * * * * *free(temp);
> diff --git a/src/pacman/util.h b/src/pacman/util.h
> index 234a631..ed03c13 100644
> --- a/src/pacman/util.h
> +++ b/src/pacman/util.h
> @@ -51,6 +51,10 @@ char *strtoupper(char *str);
> *char *strtrim(char *str);
> *char *strreplace(const char *str, const char *needle, const char *replace);
> *alpm_list_t *strsplit(const char *str, const char splitchar);
> +char *size_to_human_string(off_t bytes);
> +char *size_to_human_string_mb(off_t bytes);
> +char *size_to_human_string_kb(off_t bytes);
> +char *size_to_human_string_short(off_t bytes);
> *void string_display(const char *title, const char *string);
> *void list_display(const char *title, const alpm_list_t *list);
> *void list_display_linebreak(const char *title, const alpm_list_t *list);
> --
> 1.7.4.1
>
>
>
 
Old 02-28-2011, 03:30 PM
Jakob Gruber
 
Default Add utility functions to convert sizes to strings

On 02/25/2011 03:51 PM, Dan McGee wrote:

On Mon, Feb 21, 2011 at 1:02 PM, Jakob Gruber<jakob.gruber@gmail.com> wrote:

char *size_to_human_string(off_t bytes);
char *size_to_human_string_short(off_t bytes);

Scale to the first unit with which the value will be less or equal to
2048. size_to_human_string_short uses short unit labels ("K","M",...)
instead of the default ("KB", "MB"), uses only one decimal digit and has
no space between the value and the label.

For example, size_to_human_string: "1.31 MB", short: "1.3M".

char *size_to_human_string_mb(off_t bytes);
char *size_to_human_string_kb(off_t bytes);

Convert to MB and KB respectively and format identically to
size_to_human_string.

Signed-off-by: Jakob Gruber<jakob.gruber@gmail.com>
---
src/pacman/callback.c | 46 ++++++------------------
src/pacman/package.c | 20 ++++++-----
src/pacman/query.c | 7 ++--
src/pacman/sync.c | 7 ++--
src/pacman/util.c | 94 +++++++++++++++++++++++++++++++++++++++++--------
src/pacman/util.h | 4 ++
6 files changed, 111 insertions(+), 67 deletions(-)

diff --git a/src/pacman/callback.c b/src/pacman/callback.c
index f1e71bb..8ec4c3e 100644
--- a/src/pacman/callback.c
+++ b/src/pacman/callback.c
@@ -484,17 +484,16 @@ void cb_dl_progress(const char *filename, off_t file_xfered, off_t file_total)
{
int infolen;
int filenamelen;
- char *fname, *p;
+ char *fname, *p, *ratestr, *xferedstr;
/* used for wide character width determination and printing */
int len, wclen, wcwid, padwid;
wchar_t *wcfname;

int totaldownload = 0;
off_t xfered, total;
- double rate = 0.0, timediff = 0.0, f_xfered = 0.0;
+ double rate = 0.0, timediff = 0.0;
unsigned int eta_h = 0, eta_m = 0, eta_s = 0;
int file_percent = 0, total_percent = 0;
- char rate_size = 'K', xfered_size = 'K';

if(config->noprogressbar || file_total == -1) {
if(file_xfered == 0) {
@@ -556,7 +555,7 @@ void cb_dl_progress(const char *filename, off_t file_xfered, off_t file_total)
diff_sec = current_time.tv_sec - initial_time.tv_sec;
diff_usec = current_time.tv_usec - initial_time.tv_usec;
timediff = diff_sec + (diff_usec / 1000000.0);
- rate = xfered / (timediff * 1024.0);
+ rate = xfered / timediff;

/* round elapsed time to the nearest second */
eta_s = (int)(timediff + 0.5);
@@ -568,10 +567,10 @@ void cb_dl_progress(const char *filename, off_t file_xfered, off_t file_total)
/* return if the calling interval was too short */
return;
}
- rate = (xfered - xfered_last) / (timediff * 1024.0);
+ rate = (xfered - xfered_last) / timediff;
/* average rate to reduce jumpiness */
rate = (rate + 2 * rate_last) / 3;
- eta_s = (total - xfered) / (rate * 1024.0);
+ eta_s = (total - xfered) / (rate * 1024.0 * 1024.0);
rate_last = rate;
xfered_last = xfered;
}
@@ -625,38 +624,15 @@ void cb_dl_progress(const char *filename, off_t file_xfered, off_t file_total)

}

- /* Awesome formatting for progress bar. We need a mess of Kb->Mb->Gb stuff
- * here. We'll use limit of 2048 for each until we get some empirical */
- /* rate_size = 'K'; was set above */
- if(rate> 2048.0) {
- rate /= 1024.0;
- rate_size = 'M';
- if(rate> 2048.0) {
- rate /= 1024.0;
- rate_size = 'G';
- /* we should not go higher than this for a few years (9999.9 Gb/s?)*/
- }
- }
-
- f_xfered = xfered / 1024.0; /* convert to K by default */
- /* xfered_size = 'K'; was set above */
- if(f_xfered> 2048.0) {
- f_xfered /= 1024.0;
- xfered_size = 'M';
- if(f_xfered> 2048.0) {
- f_xfered /= 1024.0;
- xfered_size = 'G';
- /* I should seriously hope that archlinux packages never break
- * the 9999.9GB mark... we'd have more serious problems than the progress
- * bar in pacman */
- }
- }
+ ratestr = size_to_human_string_short((off_t)rate);
+ xferedstr = size_to_human_string_short(xfered);

/* 1 space + filenamelen + 1 space + 7 for size + 1 + 7 for rate + 2 for /s + 1 space + 8 for eta */
- printf(" %ls%-*s %6.1f%c %#6.1f%c/s %02u:%02u:%02u", wcfname,
- padwid, "", f_xfered, xfered_size,
- rate, rate_size, eta_h, eta_m, eta_s);
+ printf(" %ls%-*s %7s %7s/s %02u:%02u:%02u", wcfname,
+ padwid, "", xferedstr, ratestr, eta_h, eta_m, eta_s);

+ free(ratestr);
+ free(xferedstr);
free(fname);
free(wcfname);

diff --git a/src/pacman/package.c b/src/pacman/package.c
index 77a5ee7..156d257 100644
--- a/src/pacman/package.c
+++ b/src/pacman/package.c
@@ -50,7 +50,7 @@ void dump_pkg_full(pmpkg_t *pkg, int level)
{
const char *reason;
time_t bdate, idate;
- char bdatestr[50] = "", idatestr[50] = "";
+ char bdatestr[50] = "", idatestr[50] = "", *size;

I'd prefer these defined on different lines; yes they are the "same"
type but it can be confusing.

Understood, done.


const alpm_list_t *i;
alpm_list_t *requiredby = NULL, *depstrings = NULL;

@@ -105,17 +105,19 @@ void dump_pkg_full(pmpkg_t *pkg, int level)
}
list_display(_("Conflicts With :"), alpm_pkg_get_conflicts(pkg));
list_display(_("Replaces :"), alpm_pkg_get_replaces(pkg));
+
+ size = size_to_human_string_kb(alpm_pkg_get_size(pkg));
if(level< 0) {
- printf(_("Download Size : %6.2f K
"),
- (double)alpm_pkg_get_size(pkg) / 1024.0);
- }
- if(level == 0) {
- printf(_("Compressed Size: %6.2f K
"),
- (double)alpm_pkg_get_size(pkg) / 1024.0);
+ printf(_("Download Size : %s
"), size);
+ } else if(level == 0) {
+ printf(_("Compressed Size: %s
"), size);
}
+ free(size);
+
+ size = size_to_human_string_kb(alpm_pkg_get_isize(pkg));
+ printf(_("Installed Size : %s
"), size);
+ free(size);

- printf(_("Installed Size : %6.2f K
"),
- (double)alpm_pkg_get_isize(pkg) / 1024.0);
string_display(_("Packager :"), alpm_pkg_get_packager(pkg));
string_display(_("Architecture :"), alpm_pkg_get_arch(pkg));
string_display(_("Build Date :"), bdatestr);
diff --git a/src/pacman/query.c b/src/pacman/query.c
index 734875b..8a66927 100644
--- a/src/pacman/query.c
+++ b/src/pacman/query.c
@@ -247,10 +247,9 @@ static int query_search(alpm_list_t *targets)

/* print the package size with the output if ShowSize option set */
if(!config->quiet&& config->showsize) {
- /* Convert byte size to MB */
- double mbsize = (double)alpm_pkg_get_size(pkg) / (1024.0 * 1024.0);
-
- printf(" [%.2f MB]", mbsize);
+ char *size = size_to_human_string_mb(alpm_pkg_get_size(pkg));
+ printf(" [%s]", size);
+ free(size);
}


diff --git a/src/pacman/sync.c b/src/pacman/sync.c
index 7af1667..21fe4ba 100644
--- a/src/pacman/sync.c
+++ b/src/pacman/sync.c
@@ -349,10 +349,9 @@ static int sync_search(alpm_list_t *syncs, alpm_list_t *targets)

/* print the package size with the output if ShowSize option set */
if(!config->quiet&& config->showsize) {
- /* Convert byte size to MB */
- double mbsize = (double)alpm_pkg_get_size(pkg) / (1024.0 * 1024.0);
-
- printf(" [%.2f MB]", mbsize);
+ char *size = size_to_human_string_mb(alpm_pkg_get_size(pkg));
+ printf(" [%s]", size);
+ free(size);
}

if (!config->quiet) {
diff --git a/src/pacman/util.c b/src/pacman/util.c
index c08ebb1..b651478 100644
--- a/src/pacman/util.c
+++ b/src/pacman/util.c
@@ -501,10 +501,9 @@ void list_display_linebreak(const char *title, const alpm_list_t *list)
/* prepare a list of pkgs to display */
void display_targets(const alpm_list_t *pkgs, int install)
{
- char *str;
+ char *str, *size;
const alpm_list_t *i;
off_t isize = 0, dlsize = 0;
- double mbisize = 0.0, mbdlsize = 0.0;
alpm_list_t *targets = NULL;

if(!pkgs) {
@@ -522,10 +521,10 @@ void display_targets(const alpm_list_t *pkgs, int install)

/* print the package size with the output if ShowSize option set */
if(config->showsize) {
- double mbsize = (double)alpm_pkg_get_size(pkg) / (1024.0 * 1024.0);
-
- pm_asprintf(&str, "%s-%s [%.2f MB]", alpm_pkg_get_name(pkg),
- alpm_pkg_get_version(pkg), mbsize);
+ size = size_to_human_string_mb(alpm_pkg_get_size(pkg));
+ pm_asprintf(&str, "%s-%s [%s]", alpm_pkg_get_name(pkg),
+ alpm_pkg_get_version(pkg), size);
+ free(size);
} else {
pm_asprintf(&str, "%s-%s", alpm_pkg_get_name(pkg),
alpm_pkg_get_version(pkg));
@@ -533,19 +532,19 @@ void display_targets(const alpm_list_t *pkgs, int install)
targets = alpm_list_add(targets, str);
}

- /* Convert byte sizes to MB */
- mbdlsize = (double)dlsize / (1024.0 * 1024.0);
- mbisize = (double)isize / (1024.0 * 1024.0);
-
if(install) {
pm_asprintf(&str, _("Targets (%d):"), alpm_list_count(targets));
list_display(str, targets);
free(str);
printf("
");

- printf(_("Total Download Size: %.2f MB
"), mbdlsize);
+ size = size_to_human_string_mb(dlsize);
+ printf(_("Total Download Size: %s
"), size);
+ free(size);
if(!(config->flags& PM_TRANS_FLAG_DOWNLOADONLY)) {
- printf(_("Total Installed Size: %.2f MB
"), mbisize);
+ size = size_to_human_string_mb(isize);
+ printf(_("Total Installed Size: %s
"), size);
+ free(size);
}
} else {
pm_asprintf(&str, _("Remove (%d):"), alpm_list_count(targets));
@@ -553,7 +552,9 @@ void display_targets(const alpm_list_t *pkgs, int install)
free(str);
printf("
");

- printf(_("Total Removed Size: %.2f MB
"), mbisize);
+ size = size_to_human_string_mb(isize);
+ printf(_("Total Removed Size: %s
"), size);
+ free(size);

I see no reason we should lock any of these to MB units- just use the
general formatter here.

I tried not locking the size summaries, and as a user I think I'd prefer
keeping them as they are. Compare the following:


Total Download Size: 884.34 KB
Total Installed Size: 4.80 MB

vs

Total Download Size: 0.86 MB
Total Installed Size: 4.80 MB

In the latter, it seems clearer that "Installed" is larger than "Download".
Here's what it looks like with summaries locked to MB and ShowSize using
the general formatter:


Remove (1): libjpeg-8.3.0-1 [848.00 KB]

Total Removed Size: 0.83 MB

Targets (5): libjpeg-turbo-1.1.0-1 [209.01 KB]
linux-firmware-20110227-1 [8.23 MB] ncurses-5.8-1 [945.95 KB]
ppl-0.11.2-1 [2.74 MB] v4l-utils-0.8.3-1 [232.80 KB]

Total Download Size: 12.32 MB
Total Installed Size: 58.82 MB

What do you think?


}

FREELIST(targets);
@@ -594,6 +595,70 @@ static char *pkg_get_location(pmpkg_t *pkg)
}
}

+/** Converts sizes in bytes into strings.
+ *
+ * @param bytes the size in bytes
+ * @param format the format string to use. The first argument is of type
+ * float and represents the size. The second argument is of type
+ * char* and represents the unit label. for example, "%.2f %s"
+ * could result in "1.23 MB"
+ * @param unit the target unit. can be either one of the short unit
+ * labels ("B", "K", ...); or NULL, in which case the first
+ * unit which will bring the value to below a threshold of 2048
+ * will be chosen.
+ * @param long_units whether to use short ("K") or long ("KB") unit labels
+ *
+ * @return the size string
+ */
+static char *size_to_human_string_format(off_t bytes, const char *format,
+ const char *unit, int long_labels)
+{
+ char *ret;
+ static const char *shortlabels[] = {"B", "K", "M", "G", "T", "P"};
+ static const char *longlabels[] = {"B", "KB", "MB", "GB", "TB", "PB"};
+ const char **labels = long_labels ? longlabels : shortlabels;
+ const int unitcount = sizeof(shortlabels) / sizeof(shortlabels[0]);
+ int index;
+ float val = (float)bytes;
+
+ /* stop conditions:
+ * 1) a target unit is specified and we are at target unit,
+ * 2) target unit is NOT specified and value is leq 2048,
+ * and 3) we are already at the largest unit.s
+ * note that if an invalid target is specified in 1),
+ * we convert to the largest unit */
+ for(index = 0; index< unitcount - 1; index++) {
+ if(unit != NULL&& strcmp(shortlabels[index], unit) == 0) break;

I'd make the function param a char (not a "string") to ensure people
aren't doing stupid things like passing a long label in by mistake.
Then you can just use == here.


Done.

+ else if(unit == NULL&& val<= 2048.0) break;
+
+ val /= 1024.0;
+ }
+
+ pm_asprintf(&ret, format, val, labels[index]);
+
+ return ret;
+}
+
+char *size_to_human_string(off_t bytes)
+{
+ return size_to_human_string_format(bytes, "%.2f %s", NULL, 1);
+}
+
+char *size_to_human_string_mb(off_t bytes)
+{
+ return size_to_human_string_format(bytes, "%.2f %s", "M", 1);
+}
+
+char *size_to_human_string_kb(off_t bytes)
+{
+ return size_to_human_string_format(bytes, "%.2f %s", "K", 1);
+}
+
+char *size_to_human_string_short(off_t bytes)
+{
+ return size_to_human_string_format(bytes, "%.1f%s", NULL, 0);
+}
+

I think this API has been over-engineered with all these one-liners.
We aren't exposing this as a library function or anything, so people
can deal with a little complexity in the call themselves. I'd have one
function for all of this, and we can then even skip the malloc/free
cycle:
double value humanize_size(off_t bytes, char max_unit, int
long_labels, const char **label)

Logic stays as is in your format function, expect we don't actually
print the string- the value is returned and label, if provided, gets a
pointer to the correct string. Does this make sense? I'm open for
feedback of course, but I see this as 1) less boilerplate methods in
util.c 2) no free() necessary by caller.



Sure, makes sense. I changed the function as you described, but renamed
max_unit to target_unit (because that's what it actually is).



void print_packages(const alpm_list_t *packages)
{
const alpm_list_t *i;
@@ -638,8 +703,7 @@ void print_packages(const alpm_list_t *packages)
/* %s : size */
if(strstr(temp,"%s")) {
char *size;
- double mbsize = (double)pkg_get_size(pkg) / (1024.0 * 1024.0);
- pm_asprintf(&size, "%.2f", mbsize);
+ size = size_to_human_string_format(pkg_get_size(pkg), "%.2f", "M", 0);
string = strreplace(temp, "%s", size);
free(size);
free(temp);
diff --git a/src/pacman/util.h b/src/pacman/util.h
index 234a631..ed03c13 100644
--- a/src/pacman/util.h
+++ b/src/pacman/util.h
@@ -51,6 +51,10 @@ char *strtoupper(char *str);
char *strtrim(char *str);
char *strreplace(const char *str, const char *needle, const char *replace);
alpm_list_t *strsplit(const char *str, const char splitchar);
+char *size_to_human_string(off_t bytes);
+char *size_to_human_string_mb(off_t bytes);
+char *size_to_human_string_kb(off_t bytes);
+char *size_to_human_string_short(off_t bytes);
void string_display(const char *title, const char *string);
void list_display(const char *title, const alpm_list_t *list);
void list_display_linebreak(const char *title, const alpm_list_t *list);
--
1.7.4.1
 

Thread Tools




All times are GMT. The time now is 06:45 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org