Menu

#245 [LXTask] - System info - patch for feature request

open
nobody
None
5
2009-07-20
2009-07-20
DJan
No

Hi, I'm submitting here another small patch for LXTask, it "patches" this feature request :
https://sourceforge.net/tracker/index.php?func=detail&aid=1963949&group_id=180858&atid=894872

I changed it a little, I didn't use a tab but a menu item because in my opinion you don't need to check system stats that often so you don't need a tab, it would only make the feel a little bloated, so again it's in View > Show system information.

Right now it displays the OS (using uname -o, the command in the feature request doesn't work on some distros, ie Arch Linux (I have it and it returns nothing :))), the total memory in MB using the same structure as is used for the memory GtkProgressBar, CPU type, and Kernel version.

Please leave a comment with suggestions etc. :)

P.S.
If anybody has a better idea on how to find out the OS version, feel free to suggest something, because uname -o shows only GNU/Linux on some linuxes (but at least it always should show something).

Discussion

  • martyj19

    martyj19 - 2009-07-20

    It would be best to open and read /proc/version with the ordinary file API rather than using a command. Then you will not depend on how the uname command works on a particular system.

     
  • martyj19

    martyj19 - 2009-07-20

    Also, in production code you must never use a fixed size buffer into which you sprintf. This can overflow, or be a penetration path where malicious code deliberately provokes an overflow to crash you. We usually use g_strdup_printf, which is documented here, but you can select any of the functions that apply to your usage.

    http://library.gnome.org/devel/glib/stable/glib-String-Utility-Functions.html

     
  • martyj19

    martyj19 - 2009-07-20

    And finally, I would plan to write it with this API

    http://www.gnu.org/s/libc/manual/html_node/Regular-Expressions.html

    rather than invoking sed as a command.

     
  • DJan

    DJan - 2009-07-20

    Hm didn't see your last comment.

    Anyway, I fixed the sprintf and also I remove the tmp file after using it now (not a big deal, it would get removed at reboot but still more correct).

    For /proc/version, I was thinking about this, but then I switched back to uname -o, IMO still best option, for example on my system, cat /proc/version returns : "Linux version 2.6.28-ARCH (root@T-POWA-LX) (gcc version 4.3.3 (GCC) ) #1 SMP PREEMPT Sun Feb 8 10:13:45 UTC 2009" and therefore it also gives no clue as for the distribution I'm using (Arch Linux), except the kernel but that will be different in every distribution...

     
  • DJan

    DJan - 2009-07-20

    I'll check the API soon

     
  • martyj19

    martyj19 - 2009-07-20

    I suggest the system call version of uname (man 2 uname) might be of help.

     
  • DJan

    DJan - 2009-07-20

    Didn't know that there is a system call of uname, and read the comment too late (slow internet, it takes ages to reload so I'm not reloading often).

    However I uploaded a new version of the patch, I think it does the job, and no calls to system() and no external commands are made, I get the CPU info using the regex.h, and the OS and kernel in /proc/sys/kernel/ostype and osrelease.

     
  • martyj19

    martyj19 - 2009-07-20

    All of the returns leak a widget and at least one of them leaks an open file. Also, you haven't fixed the potential overflow of the sprintf buffers. The comment that says stdio is being included for tmpnam is no longer correct.

    As a maintenance matter, it is better to use sizeof() than repeat the dimensionality of an array further down. Also, it is better to move the declaration of a variable down to where it is initialized. The days when you had to declare all variables at the top of a function are long gone.

    After you have these issues fixed and you have tested thoroughly, I suggest you join the mailing list at https://lists.sourceforge.net/lists/listinfo/lxde-list and ask people to test and evaluate your patch.

     
  • DJan

    DJan - 2009-07-20

    v4

     
  • DJan

    DJan - 2009-07-20

    OK, I fixed it. However for cpu1 I couldn't use sizeof() as it's dynamically allocated, should I leave it as it is, or maybe use a #define to define the size?
    Also I sent it to the subscribers of the list.

     
  • DJan

    DJan - 2009-07-21

    As PCMan suggested, I'm now using <glib gregex.h=""> instead of <regex.h>

     
  • Nobody/Anonymous

    when we see patch in mainline lxtask?

     
  • DJan

    DJan - 2009-08-22

    Well I don't have submit rights to SVN so I can't put it in, but I think it's ready now, I sent it to the list and nobody complained (except using a different lib for regexes) about anything, so maybe someone with a SVN account can you submit it?

    Thanks..

     
  • Nobody/Anonymous

    Status of this patch?

     

Log in to post a comment.