Re: [Lxde-list] [PATCH] handling of memory sizes in lxtask

[Henry G. <hsg...@go...>]

On Sun, Jul 22, 2012 at 05:31:03AM +0200, John Spencer wrote:
> ...

Nice, I have one comment about your second patch:

> >From 5e5b4b300f9bdd937dc220e178e675fa5723308d Mon Sep 17 00:00:00 2001
> From: John Spencer <mai...@ba...>
> Date: Sun, 22 Jul 2012 05:10:28 +0200
> Subject: [PATCH 2/3] fix PAGE_SIZE symbol name to not collide with the macro of the same name, and to mean size in bytes unlike in the previous misleading meaning as page_size_in_kbytes. remove unnecessary scaling.
> 
> ---
>  src/interface.c              |    6 +++---
>  src/main.c                   |    4 ++--
>  src/types.h                  |    2 +-
>  src/xfce-taskmanager-linux.c |    4 ++--
>  4 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/src/interface.c b/src/interface.c
> index 45cf256..824ec3a 100644
> --- a/src/interface.c
> +++ b/src/interface.c
> @@ -443,11 +443,11 @@ void fill_list_item(gint i, GtkTreeIter *iter)
>          gtk_tree_store_set(GTK_TREE_STORE(list_store), iter, COLUMN_PPID, buf, -1);
>          gtk_tree_store_set(GTK_TREE_STORE(list_store), iter, COLUMN_STATE, task->state, -1);
>  
> -        /* size */
> -        gtk_tree_store_set(GTK_TREE_STORE(list_store), iter, COLUMN_MEM, size_to_string(buf, ((guint64)task->size)*1024), -1);
> +        /* vm size */
> +        gtk_tree_store_set(GTK_TREE_STORE(list_store), iter, COLUMN_MEM, size_to_string(buf, task->size), -1);
>  
>          /* rss */
> -        gtk_tree_store_set(GTK_TREE_STORE(list_store), iter, COLUMN_RSS, size_to_string(buf, ((guint64)task->rss)*1024), -1);
> +        gtk_tree_store_set(GTK_TREE_STORE(list_store), iter, COLUMN_RSS, size_to_string(buf, task->rss), -1);
>  
>          gtk_tree_store_set(GTK_TREE_STORE(list_store), iter, COLUMN_UNAME, task->uname, -1);
>          sprintf(buf,"%0d%%", (guint)task->time_percentage);
> diff --git a/src/main.c b/src/main.c
> index 0421387..f79e7af 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -57,7 +57,7 @@ guint win_height;
>  guint refresh_interval;
>  guint rID;
>  
> -int PAGE_SIZE;
> +int page_size;

Shouldn't this be a long, as sysconf returns that?

>  
>  int main (int argc, char *argv[])
>  {
> @@ -73,7 +73,7 @@ int main (int argc, char *argv[])
>  #endif
>      gtk_init (&argc, &argv);
>  
> -    PAGE_SIZE=sysconf(_SC_PAGESIZE)>>10;
> +    page_size = sysconf(_SC_PAGESIZE);
>      own_uid = getuid();
>  
>      config_file = g_build_filename(g_get_user_config_dir(), "lxtask.conf", NULL);
> diff --git a/src/types.h b/src/types.h
> index 5c1e644..a99dfb4 100644
> --- a/src/types.h
> +++ b/src/types.h
> @@ -81,6 +81,6 @@ extern guint win_height;
>  const gchar *custom_signal_0;
>  const gchar *custom_signal_1;
>  
> -extern int PAGE_SIZE;
> +extern int page_size;

Again, long?

>  
>  #endif
> diff --git a/src/xfce-taskmanager-linux.c b/src/xfce-taskmanager-linux.c
> index 443d3d1..d182486 100644
> --- a/src/xfce-taskmanager-linux.c
> +++ b/src/xfce-taskmanager-linux.c
> @@ -46,8 +46,8 @@ void get_task_details(gint pid,struct task *task)
>  	sscanf(line,"%zu %zu",&task->size,&task->rss);
>  	close(fd);
>  	if(!task->size) return;
> -	task->size*=PAGE_SIZE;
> -	task->rss*=PAGE_SIZE;
> +	task->size *= page_size;
> +	task->rss *= page_size;
>  
>  	sprintf(line,"/proc/%d/stat",pid);
>  	fd=open(line,O_RDONLY);
> -- 
> 1.7.3.4
> 

Thanks,

Henry