Gentoo Forums
Gentoo Forums
Gentoo Forums
Quick Search: in
too many and too less terminal-emulators
View unanswered posts
View posts from last 24 hours

Goto page Previous  1, 2, 3  
Reply to topic    Gentoo Forums Forum Index Off the Wall
View previous topic :: View next topic  
Author Message
Fran
Guru
Guru


Joined: 29 Feb 2004
Posts: 521
Location: Coruña (Spain)

PostPosted: Tue Jan 29, 2013 9:08 am    Post subject: Reply with quote

Dr.Willy wrote:
memset superfluous! strlen() superfluous! PERFORMENSE!

Yeah, come on, snprintf returns the number of chars written, so you can do a
Code:
lastpos += snprintf(st + lastpos,...

and avoid memset + strlen. That's what I do in mine. The diference is quite noticeable, I shaved 5.27 nanoseconds per second :P
Back to top
View user's profile Send private message
Bones McCracker
Veteran
Veteran


Joined: 14 Mar 2006
Posts: 1563
Location: U.S.A.

PostPosted: Tue Jan 29, 2013 9:49 am    Post subject: Reply with quote

Dr.Willy wrote:
BoneKracker wrote:
Code:

URxvt*color0: #000010       // black -> black russian
URxvt*color1: #9e1828       // red -> tamarillo
URxvt*color2: #aece92       // green -> pine glade
URxvt*color3: #968a38       // yello -> sycamore
URxvt*color4: #414171       // blue -> east bay
URxvt*color5: #963c59       // magenta -> vin rouge
URxvt*color6: #418179       // cyan -> viridian
URxvt*color7: #bebebe       // white -> silver
URxvt*color8: #666666       // bright black -> dove gray
URxvt*color9: #cf6171       // bright red -> chestnut rose
URxvt*color10: #c5f779      // gright green -> sulu
URxvt*color11: #fff796      // bright yellow -> witch haze
URxvt*color12: #4186be      // bright blue -> steel blue
URxvt*color13: #cf9ebe      // bright magenta -> viola
URxvt*color14: #71bebe      // bright cyan -> neptune
URxvt*color15: #ffffff      // bright white -> white
URxvt*background: 0
URxvt*foreground: 15
URxvt*cursorColor: magenta


…if you wrote those comments yourself, it's time to hand in your man-card. :P

No seriously, those are actually really good. cat >> .Xresources I say.

I picked the colors, I didn't name them. :lol:

Dr.Willy wrote:
BoneKracker wrote:
Rather than a bash script, this is what I use to produce my "status bar", which is updated every second and looks like this:
Code:
Load: 0.46   RAM: 32 MiB   Swap: 0 MiB    Thu Jan 24   00:00:18


This is the code. I wrote it for my use only. It's not very portable, flexible, or idiot-proof; it doesn't include error checking and the other things good code should have before it's distributed. But it's far more efficient than a shell script (the normal way of doing it). It also never closes the display, but that doesn't matter because I always close X when close dwm. To be "correct", I suppose the pointer to the XDisplay should be a global variable, so it could be closed by signal trapping, but I hate global variables and that scenario will probably never occur, and it's a one-time 4-byte leak if it did occur, so fuck it. :lol:

No!
Must -fomg-optimize!
Xlib bad! memset superfluous! strlen() superfluous! PERFORMENSE!

Okay, look, I'm a novice C programmer. This is my first C program that's actually useful in any way. So go easy on me.

Now, is there a better way to access the X interface than Xlib? How should I have done it?

The memset might be unnecessary, but there are cases where I could theoretically end up with trailing characters from the previous iteration still showing up. status is a char buffer, and XStoreName technically takes a struct called "XTextProperty", which I didn't bother to look at. I'm operating under the assumption that it will not stop at the first newline and would print everything in the buffer. So the buffer should be cleared each iteration, no? The way I saw it, the important thing was that I am re-using the same memory over and over again, avoiding having to constantly allocate new memory each iteration. That's why I used "static" for the vars in the functions, too. It's my understanding this has that same effect.

I don't see Strlen as superfluous. I could have used a 'strings' library function to append the string, but it wouldn't have cleared the existing newline. So the alternative would have been to first delete the terminating newline, then append the string, or do what I did, which just happens to have the beneficial side-effect of overwriting the terminating null.

Are you making fun of me because my code is amateurish and reeks of sophomoric over-optimization or something? :oops: I'm a novice, so I wouldln't be surprised if it gave that impression. If so, please give me some more detailed mentorship on what I should change.

I don't think replacing a shell script with this compiled C code is overkill, though. The script was taking about one third of a second to run, every second. This runs in 0.005 seconds. Also, when I tried to add an IMAP Idle listener to my script, I was running into a problem achieving non-blocking interprocess communication, which is not a strong point of shell scripting. Furthermore, Given the nature of the program, it wasn't much more complex in C, and I don't think it would be much simpler in something like Python.

Dr.Willy wrote:
BoneKracker wrote:
To be honest, C is overkill for most things a user would want to do, but extending your desktop environment can be an exception to that, if it's a function that's constantly running or very frequently used.

Yeah that reminds me of when I was still using wmii. I mean I really liked it for what it did, it came with nice defaults and I also liked the "programmable configuration".
But as much as I liked it's 9p interface, when I realised that I was effectively running a shellscript that started a python program that read the user input and used a python library to write commands to a file that would be handeled by a 9p server that would interpret the command I felt that this might just be a little too much overhead for opening a terminal.

-funroll-all-teh-loops !

Everything-is-a-file does enable a great deal of integration flexibility, though. But sometimes, it's suboptimal. The memory function here is a good example. My C program is reading a file that's pushed into userspace by the kernel. It would somewhat more efficient in this case if the kernel exposed a function I could use to get the same data. On the other hand, that file is accessible by a hundred different more user-friendly means (e.g., read, sed, awk, etc., etc.).

Seriously, though. I'd welcome any feedback on my little C program. Like I said, I'm aware that it could be far more portable and have a lot more error-checking.

One question I had in particular was this:

Because of the infinite loop, the following is effectively dead code (never reached):
Code:
  XCloseDisplay(dpy);

  return 0;

Now, in reality, unless this program or the window manager crashes, this program is always going to be terminated by my X session being closed (as in a reboot). But, technically, I think this leaves dpy, which is nothing but a pointer, unrecovered, and I think it leaves the XDisplay struct that it points to also unrecovered, does it not?

I started to write a signal-trapping function to catch SIGSTOP and SIGTERM (which would execute XCloseDisplay and free the pointer), but I can't really do that unless I make dpy a global variable (or pass it to each and every function, most of which don't really need it). But I have this misgiving about global variables, probably from having started with C++ before starting to learn C.

So, what's the best way to handle this? Should I make dpy a global variable (i.e., execute the XOpenDisplay in global namespace, so that, in the event of an error condition or user-generation stop or term, I can then execute XCloseDisplay in global namespace)?
_________________
True Liberals are individualists. Democrats, on the other hand, are authoritarian collectivists.
Back to top
View user's profile Send private message
Fran
Guru
Guru


Joined: 29 Feb 2004
Posts: 521
Location: Coruña (Spain)

PostPosted: Tue Jan 29, 2013 10:50 am    Post subject: Reply with quote

BoneKracker wrote:
The memset might be unnecessary, but there are cases where I could theoretically end up with trailing characters from the previous iteration still showing up.

That's why you add a '\0':
Code:
 append_date();
 st[lastpos] = '\0';

BoneKracker wrote:
I don't see Strlen as superfluous. I could have used a 'strings' library function to append the string, but it wouldn't have cleared the existing newline.

See my comment above regarding snprintf. lastpos is a global variable.
Back to top
View user's profile Send private message
Dr.Willy
Guru
Guru


Joined: 15 Jul 2007
Posts: 323
Location: NRW, Germany

PostPosted: Tue Jan 29, 2013 10:58 am    Post subject: Reply with quote

BoneKracker wrote:
Now, is there a better way to access the X interface than Xlib? How should I have done it?

http://xcb.freedesktop.org/Features/

BoneKracker wrote:
The memset might be unnecessary, but there are cases where I could theoretically end up with trailing characters from the previous iteration still showing up. status is a char buffer, and XStoreName technically takes a struct called "XTextProperty", which I didn't bother to look at. I'm operating under the assumption that it will not stop at the first newline and would print everything in the buffer. So the buffer should be cleared each iteration, no? The way I saw it, the important thing was that I am re-using the same memory over and over again, avoiding having to constantly allocate new memory each iteration. That's why I used "static" for the vars in the functions, too. It's my understanding this has that same effect.

C terminates strings with a '\0', no matter how long your buffer actually is. That means that everything that follows after the '\0' is irrelevant and will be ignored.
{'f', 'o', 'o', '\0', 'b', 'a', 'r'} or {'f', 'o', 'o', '\0', '\0', '\0', '\0'} both results in "foo".

BoneKracker wrote:
I don't see Strlen as superfluous. I could have used a 'strings' library function to append the string, but it wouldn't have cleared the existing newline. So the alternative would have been to first delete the terminating newline, then append the string, or do what I did, which just happens to have the beneficial side-effect of overwriting the terminating null.

What strlen() (or strcat or whatever string function that needs to know the end of the string) does is start at the pointer and loop through it until it finds a '\0'.
You need that to get to the position where you want to append the other messages. But you can also get that position from the snprintf() function, which returns the number of characters written.

So
Code:
int len = 0;
len += snprintf(&buf[len], SIZE-len, "B: ");
len += sprint_battery(&buf[len], SIZE-len, "/sys/class/power_supply/BAT0");
len += snprintf(&buf[len], SIZE-len, "|");
len += snprintf(&buf[len], SIZE-len, "L: ");
len += sprint_loadavg(&buf[len], SIZE-len);
len += snprintf(&buf[len], SIZE-len, "|");
len += sprint_time(&buf[len], SIZE-len, "%a %d %b %H:%M|KW %W");
//snprintf(&buf[len], SIZE-len, " => %d", len);

set_status(buf);

(ofc, it can also return -1, blahblah I don't care)

BoneKracker wrote:
Are you making fun of me because my code is amateurish and reeks of sophomoric over-optimization or something? :oops: I'm a novice, so I wouldln't be surprised if it gave that impression. If so, please give me some more detailed mentorship on what I should change.

Nah.
Overoptimizing pet projects is fine :lol:

BoneKracker wrote:
Everything-is-a-file does enable a great deal of integration flexibility, though.

With 9p you could even send all that shit over the network.

BoneKracker wrote:
I started to write a signal-trapping function to catch SIGSTOP and SIGTERM (which would execute XCloseDisplay and free the pointer), but I can't really do that unless I make dpy a global variable (or pass it to each and every function, most of which don't really need it). But I have this misgiving about global variables, probably from having started with C++ before starting to learn C.

So, what's the best way to handle this? Should I make dpy a global variable (i.e., execute the XOpenDisplay in global namespace, so that, in the event of an error condition or user-generation stop or term, I can then execute XCloseDisplay in global namespace)?

There are many reasons why global variables are bad. One is that it leads to a bad program structure: If you keep variables local you will realise if things are in the wrong place and if you find yourself writing functions that take 7 arguments that have nothing to do with each other, that is where you must realize you're doing something stupid. With everything global you might never know.
But if stuff needs to be global, then so be it. (Strictly speaking you're not even introducing a global variable. It only needs be visible inside your module which would make it comparable to a field in a class).
Anyway. You're getting a message from the 'outside' (i.e. the signal) so you have to provide some switches for the outside to control your program.
You want to make your program exit gracefully. So you could either make dpy and whatever global and let the outside clean up that stuff or just tell your program that it's done now. In your case you need to exit the main loop.
Code:
while(1) {} --> while(running) {}


Fran wrote:
That's why you add a '\0':
Code:
 append_date();
 st[lastpos] = '\0';

man snprintf wrote:
The functions snprintf() and vsnprintf() do not write more than size bytes (including the terminating null byte ('\0')).


Fran wrote:
Code:
 append_date();
 st[lastpos] = '\0';

See my comment above regarding snprintf. lastpos is a global variable.

Why would you make that global?
Back to top
View user's profile Send private message
Bones McCracker
Veteran
Veteran


Joined: 14 Mar 2006
Posts: 1563
Location: U.S.A.

PostPosted: Tue Jan 29, 2013 11:34 am    Post subject: Reply with quote

Fran wrote:
Dr.Willy wrote:
memset superfluous! strlen() superfluous! PERFORMENSE!

Yeah, come on, snprintf returns the number of chars written, so you can do a
Code:
lastpos += snprintf(st + lastpos,...

and avoid memset + strlen. That's what I do in mine. The diference is quite noticeable, I shaved 5.27 nanoseconds per second :P

Ah, good idea! strftime also returns size_t. So I can just sum the return values from each function as the offset for the next function.

Code:
call function 1

offset = return value of function 1

call function 2 (pass offset)

offset = offest + return value of function 2

call function 3 (pass offset)


Hmmm... is that really more efficient than just taking the strelen before appending the output? Now I'm having to store an additional variable (short int or char), and pass it to two functions, and I'm having to pass back return values from two functions. The string is only a couple of dozen bytes long. Can this really be more efficient that just getting the strlen?
_________________
True Liberals are individualists. Democrats, on the other hand, are authoritarian collectivists.
Back to top
View user's profile Send private message
Fran
Guru
Guru


Joined: 29 Feb 2004
Posts: 521
Location: Coruña (Spain)

PostPosted: Tue Jan 29, 2013 11:42 am    Post subject: Reply with quote

BoneKracker wrote:
Can this really be more efficient that just getting the strlen?

Yeah. One needs a loop, the other doesn't. And if you do as I do and use a global variable for lastpos, you don't need to pass it around. (Seriously, if you are passing a variable to ALL the functions in your program, just use a global. K&R won't come from their tombs to kick your ass.)
Back to top
View user's profile Send private message
Dr.Willy
Guru
Guru


Joined: 15 Jul 2007
Posts: 323
Location: NRW, Germany

PostPosted: Tue Jan 29, 2013 11:54 am    Post subject: Reply with quote

Fran wrote:
K&R won't come from their tombs to kick your ass.

So you say. Then again, K isn't even dead yet. BETTER SAFE THAN SORRY.
Back to top
View user's profile Send private message
Fran
Guru
Guru


Joined: 29 Feb 2004
Posts: 521
Location: Coruña (Spain)

PostPosted: Tue Jan 29, 2013 11:56 am    Post subject: Reply with quote

Dr.Willy wrote:

man snprintf wrote:
The functions snprintf() and vsnprintf() do not write more than size bytes (including the terminating null byte ('\0')).

Good. No need to append it then :)

Dr.Willy wrote:
Why would you make that global?

Clearer and simpler code. All functions need it. No need to return it and pass it around.

So, why not? ;) Globals are like gotos: generally bad, but sometimes they are the best solution.
Back to top
View user's profile Send private message
Bones McCracker
Veteran
Veteran


Joined: 14 Mar 2006
Posts: 1563
Location: U.S.A.

PostPosted: Tue Jan 29, 2013 12:15 pm    Post subject: Reply with quote

Dr.Willy wrote:
BoneKracker wrote:
Now, is there a better way to access the X interface than Xlib? How should I have done it?

http://xcb.freedesktop.org/Features/

I'm aware of the existence of xcb and I'm using xcb on my system everywhere USE flags allow it. A cursory look at it horrified me, though and I had a good Xlib example to follow. But I'll look at it again.

Dr.Willy wrote:
BoneKracker wrote:
The memset might be unnecessary, but there are cases where I could theoretically end up with trailing characters from the previous iteration still showing up. status is a char buffer, and XStoreName technically takes a struct called "XTextProperty", which I didn't bother to look at. I'm operating under the assumption that it will not stop at the first newline and would print everything in the buffer. So the buffer should be cleared each iteration, no? The way I saw it, the important thing was that I am re-using the same memory over and over again, avoiding having to constantly allocate new memory each iteration. That's why I used "static" for the vars in the functions, too. It's my understanding this has that same effect.

C terminates strings with a '\0', no matter how long your buffer actually is. That means that everything that follows after the '\0' is irrelevant and will be ignored.

{'f', 'o', 'o', '\0', 'b', 'a', 'r'} or {'f', 'o', 'o', '\0', '\0', '\0', '\0'} both results in "foo".

Yes, I understand that (btw, I said 'newline' earlier but I meant 'null'), but it depends on how it is handled. My thinking was that there's no guarantee a char arrays is going to be treated as a string. Now, granted, I am populating it with a string, so it does indeed contain a terminating null, but I was uncertain whether the function the pointer is passed to (XStoreName) is treating it as a string or a buffer (it's expecting an "XTextProperty", whatever that is). I thought about this, and even after looking briefly at its man page and source, I wasn't sure, so I erred on the side of caution and cleared the array. Now that I think about it, though, I suppose it MUST be treating it as a string, because there is no size parameter. Unless it uses the null as a terminator, it would require a size parameter.

Okay, I can remove my memset. Thank you, this is good advice. :)

Dr.Willy wrote:
BoneKracker wrote:
I don't see Strlen as superfluous. I could have used a 'strings' library function to append the string, but it wouldn't have cleared the existing null. So the alternative would have been to first delete the terminating null, then append the string, or do what I did, which just happens to have the beneficial side-effect of overwriting the terminating null.

What strlen() (or strcat or whatever string function that needs to know the end of the string) does is start at the pointer and loop through it until it finds a '\0'.
You need that to get to the position where you want to append the other messages. But you can also get that position from the snprintf() function, which returns the number of characters written.

So
Code:
int len = 0;
len += snprintf(&buf[len], SIZE-len, "B: ");
len += sprint_battery(&buf[len], SIZE-len, "/sys/class/power_supply/BAT0");
len += snprintf(&buf[len], SIZE-len, "|");
len += snprintf(&buf[len], SIZE-len, "L: ");
len += sprint_loadavg(&buf[len], SIZE-len);
len += snprintf(&buf[len], SIZE-len, "|");
len += sprint_time(&buf[len], SIZE-len, "%a %d %b %H:%M|KW %W");
//snprintf(&buf[len], SIZE-len, " => %d", len);

set_status(buf);

(ofc, it can also return -1, blahblah I don't care)

Okay, that's useful advice -- I got that from Fran's comment above. But is that really more efficient (see my response to him)? Is it really more efficient to store an additional variable and pass it four times than to run strlen twice on a short array? Perhaps, but it can't be by much. Then again, I can see that the fact that these standard string functions return a size is a hint that this is a good design pattern (or at least a customary one), so I'll take the hint and adopt this. Thanks again. :)


Dr.Willy wrote:
BoneKracker wrote:
Are you making fun of me because my code is amateurish and reeks of sophomoric over-optimization or something? :oops: I'm a novice, so I wouldln't be surprised if it gave that impression. If so, please give me some more detailed mentorship on what I should change.

Nah.
Overoptimizing pet projects is fine :lol:

It's more that it's CONSTANTLY running. Did I really over-optimize something here? The only thing I can think of is that I passed a pointer to a single buffer to the functions to append to, rather than having the functions return a string.

Dr.Willy wrote:
BoneKracker wrote:
Everything-is-a-file does enable a great deal of integration flexibility, though.

With 9p you could even send all that shit over the network.

I used wmii for a while before dwm. When I realized that, I had all kinds of lightbulbs going on over my head. I never did anything with it, though. :lol:

I don't follow your "while(running)" suggestion. Thank you both for the two other suggestions.
_________________
True Liberals are individualists. Democrats, on the other hand, are authoritarian collectivists.


Last edited by Bones McCracker on Tue Jan 29, 2013 12:29 pm; edited 2 times in total
Back to top
View user's profile Send private message
Dr.Willy
Guru
Guru


Joined: 15 Jul 2007
Posts: 323
Location: NRW, Germany

PostPosted: Tue Jan 29, 2013 12:16 pm    Post subject: Reply with quote

Fran wrote:
Dr.Willy wrote:
Why would you make that global?

Clearer and simpler code. All functions need it. No need to return it and pass it around.

So, why not? ;) Globals are like gotos: generally bad, but sometimes they are the best solution.

Mh, I found that I'd rather pass the argument to every function instead. Helps me keep an overview over things. That or I'm writing too much Haskell lately.
Back to top
View user's profile Send private message
Dr.Willy
Guru
Guru


Joined: 15 Jul 2007
Posts: 323
Location: NRW, Germany

PostPosted: Tue Jan 29, 2013 12:32 pm    Post subject: Reply with quote

BoneKracker wrote:
I'm aware of the existence of xcb and I'm using xcb on my system everywhere USE flags allow it. A cursory look at it horrified me, though and I had a good Xlib example to follow. But I'll look at it again.
[…]
I don't follow your "while(running)" suggestion. Thank you both for the two other suggestions.

Fuck explainations, lets talk code:
Code:
#define _POSIX_C_SOURCE 200112L

#include <unistd.h>
#include <stdbool.h>
#include <stdio.h>
//#include <stdlib.h>
#include <time.h>

#include <signal.h>

#include <xcb/xcb.h>
#include <xcb/xcb_aux.h>
#include <xcb/xcb_icccm.h>

#define ERRSTR "err"

static bool running;

size_t
sprint_time(char *statusbuf, const size_t size, const char *fmt) {
   time_t now;
   struct tm *ltm;

   now = time(NULL);
   ltm = localtime(&now);

   return strftime(statusbuf, size, fmt, ltm);
}

size_t
sprint_loadavg(char *statusbuf, const size_t size) {
   double avgs[3];

   if(getloadavg(avgs, 3) < 0) {
      return snprintf(statusbuf, size, ERRSTR);
   }

   return snprintf(statusbuf, size, "%.2f %.2f %.2f", avgs[0], avgs[1], avgs[2]);
}

/*
 * Linux seems to change the filenames after suspend/hibernate
 * according to a random scheme. So just check for both possibilities.
 */
size_t
sprint_battery(char *statusbuf, const size_t size, const char *basedir) {
   int present = 0;
   int descap = -1;
   int remcap = -1;
   FILE *fd;

   if(chdir(basedir) == -1)
      return snprintf(statusbuf, size, ERRSTR);

   if((fd = fopen("present", "r")) != NULL) {
      fscanf(fd, "%d", &present);
      fclose(fd);
      if(present == 0) {
         return snprintf(statusbuf, size, "-");
      }
   } else {
      return snprintf(statusbuf, size, ERRSTR);
   }

   if((fd = fopen("charge_full_design", "r")) != NULL
   || (fd = fopen("energy_full_design", "r")) != NULL) {
      fscanf(fd, "%d", &descap);
      fclose(fd);
   } else {
      return snprintf(statusbuf, size, ERRSTR);
   }

   if((fd = fopen("charge_now", "r")) != NULL
   || (fd = fopen("energy_now", "r")) != NULL) {
      fscanf(fd, "%d", &remcap);
      fclose(fd);
   } else {
      return snprintf(statusbuf, size, ERRSTR);
   }

   if(remcap <= 0 || descap <= 0)
      return snprintf(statusbuf, size, ERRSTR);

   return snprintf(statusbuf, size, "%.1f%%", ((float)remcap / (float)descap) * 100);
}

void
sighandler(int signum) {
   running = false;
}

int
main(void) {
   struct sigaction sa;
   sa.sa_handler = &sighandler;
   sigemptyset(&sa.sa_mask);
   if(sigaction(SIGTERM, &sa, NULL) < 0 || sigaction(SIGINT, &sa, NULL) < 0) {
      fprintf(stderr, "cannot set signal handler\n");
      return 1;
   }

   xcb_connection_t *c;
   xcb_screen_t *screen;
   int screen_num;

   c = xcb_connect(NULL, &screen_num);
   screen = xcb_aux_get_screen(c, screen_num);
   /*
   xcb_screen_iterator_t iter = xcb_setup_roots_iterator(xcb_get_setup(c));
   for (i = 0; i < screen_num; ++i) { // we want the screen at index screen_num of the iterator
      xcb_screen_next(&iter);
   }
   screen = iter.data;
   */

   const size_t SIZE = 128;
   char buf[SIZE];

   for(running = true; running; sleep(10)) {
      if(xcb_connection_has_error(c)) {
         fprintf(stderr, "lost connection to xserver\n");
         return 1;
      }

      int len = 0;
      len += snprintf(&buf[len], SIZE-len, "B: ");
      len += sprint_battery(&buf[len], SIZE-len, "/sys/class/power_supply/BAT0");
      len += snprintf(&buf[len], SIZE-len, "|L: ");
      len += sprint_loadavg(&buf[len], SIZE-len);
      len += snprintf(&buf[len], SIZE-len, "|");
      len += sprint_time(&buf[len], SIZE-len, "%a %d %b %H:%M|KW %W");
      //snprintf(&buf[len], SIZE-len, " => %d", len);

      xcb_icccm_set_wm_name(c, screen->root, XCB_ATOM_STRING, 8, len, buf);
      xcb_flush(c);
   }
   xcb_icccm_set_wm_name(c, screen->root, XCB_ATOM_STRING, 8, 3, "---");
   xcb_flush(c);

   xcb_disconnect(c);

   return 0;
}

Not the cleanest of codes either and I'm not even sure xcb_connection_has_error() does what I hope it does, but anyway.

BoneKracker wrote:
Okay, that's useful advice -- I got that from Fran's comment above. But is that really more efficient (see my response to him)? Is it really more efficient to store an additional variable and pass it four times than to run strlen twice on a short array? Perhaps, but it can't be by much. Then again, I can see that the fact that these standard string functions return a size is a hint that this is a good design pattern (or at least a customary one), so I'll take the hint and adopt this. Thanks again. :)

You dont have to pass it. All you need to do is replace the strlen() calls.
And yes, if you can replace non-trivial computations (i.e. loops, lookups, …) with a variable its almost certainly a good idea.
Back to top
View user's profile Send private message
Bones McCracker
Veteran
Veteran


Joined: 14 Mar 2006
Posts: 1563
Location: U.S.A.

PostPosted: Tue Jan 29, 2013 12:35 pm    Post subject: Reply with quote

The instructional material I've read (which is more C++ oriented) frowns on global variables, even disparages and castigates them: evil, evil, evil!

I see them in C code, though. I myself wouldn't have thought of using it, except that when I tried to write signal handling code, it didn't have visibility of this particular thing I wanted to clean up.

Now, I didn't spend much time (like 10 minutes) fiddling with that before giving up (for now). It seems to me there must be a way of passing what's needed to the signal handling function; I just don't know how to do it yet.
_________________
True Liberals are individualists. Democrats, on the other hand, are authoritarian collectivists.
Back to top
View user's profile Send private message
Bones McCracker
Veteran
Veteran


Joined: 14 Mar 2006
Posts: 1563
Location: U.S.A.

PostPosted: Tue Jan 29, 2013 1:00 pm    Post subject: Reply with quote

Ah, you must be a dwm (or similar) user as well. :lol:

Thank you, this will be most helpful, especially pertaining to the xcb stuff. I had looked at a couple of other people's similar C programs for dwm status but even I could tell they sucked, so I got little of value from them except a hint at using Xlib.

While I am incorporating these two ideas (and I'll probably also switch to xcb and borrow heavily from your signal handling, if you don't mind), I had one other question while I was writing this.

I am only interested in the current (1 minute) load average. My function:
Code:
// Reads system load (1 min avg) and appends to passed status string.                                   
static size_t append_load(char *st) {

  static const size_t max = 13;
  static double loads[1];

  getloadavg(loads, 1);

  return snprintf(st, max, "  Load: %.2f ", loads[0]);

}

Declares an array of doubles, of scale 1 (i.e. one member). It does this because the getloadavg() function expects to be passed a pointer to an array of doubles. Could I have just passed it a pointer to a double and ignored any warnings (which I assume I'd have generated), and would there be any efficiency in doing so? I assumed this would be bad practice and not really achieve any gains (an array really just being an abstraction and not having any real overhead), so I didn't try it. Reassure me I did the right thing.
_________________
True Liberals are individualists. Democrats, on the other hand, are authoritarian collectivists.
Back to top
View user's profile Send private message
Bones McCracker
Veteran
Veteran


Joined: 14 Mar 2006
Posts: 1563
Location: U.S.A.

PostPosted: Tue Jan 29, 2013 1:29 pm    Post subject: Reply with quote

I have another question (or Dr. Willy):

Is there any particular reason why you're passing (&buf[len]) as opposed to (buf + len)?

An array name is a pointer variable, so you're forcing it to identify the address of a position within the array. It seems to me it would be more efficient simply to use pointer arithmetic and add the offset to the base pointer (i.e. just pass (buf + len) )?
_________________
True Liberals are individualists. Democrats, on the other hand, are authoritarian collectivists.
Back to top
View user's profile Send private message
Fran
Guru
Guru


Joined: 29 Feb 2004
Posts: 521
Location: Coruña (Spain)

PostPosted: Tue Jan 29, 2013 1:42 pm    Post subject: Reply with quote

BoneKracker wrote:
Declares an array of doubles, of scale 1 (i.e. one member). It does this because the getloadavg() function expects to be passed a pointer to an array of doubles.

No, it expects a pointer to double.

BoneKracker wrote:
Could I have just passed it a pointer to a double and ignored any warnings (which I assume I'd have generated), and would there be any efficiency in doing so? I assumed this would be bad practice and not really achieve any gains (an array really just being an abstraction and not having any real overhead), so I didn't try it. Reassure me I did the right thing.

No problem. No warnings will be generated. I have this in mine:
Code:
        double loadavg;
        if (getloadavg(&loadavg, 1) == -1) return;
        c = loadavg < 1 ? normal : (loadavg < 3 ? yellow : red);
        lastpos += sprintf(st + lastpos, "  %c\uE01A %-4.2f", c, loadavg);

(yeah, I like colors :P).

BoneKracker wrote:
Is there any particular reason why you're passing (&buf[len]) as opposed to (buf + len)?

Cosmetic, I assume. I like buf + len. I remember finding the asm code was different years ago, but I don't remember why. Had something to do with 64 bit and 32 bit. Anyway, both will do the same.

(edit) lol, found it: http://www.elotrolado.net/hilo_p-i-vs-p-i_460715

In spanish, but you can see the asm fragments. That was in 2005. I don't know if gcc still treats buf[len] and *(buf + len) differently.


Last edited by Fran on Tue Jan 29, 2013 1:52 pm; edited 1 time in total
Back to top
View user's profile Send private message
Prenj
n00b
n00b


Joined: 20 Nov 2011
Posts: 13

PostPosted: Tue Jan 29, 2013 1:52 pm    Post subject: Reply with quote

Fran wrote:
Dr.Willy wrote:
memset superfluous! strlen() superfluous! PERFORMENSE!

Yeah, come on, snprintf returns the number of chars written, so you can do a
Code:
lastpos += snprintf(st + lastpos,...

and avoid memset + strlen. That's what I do in mine. The diference is quite noticeable, I shaved 5.27 nanoseconds per second :P


Meh. Just use glib2.

char* s = g_new0(len); (malloc+memset)
or even better

g_string: http://developer.gnome.org/glib/2.28/glib-Strings.html

it's still C but you don't have to fiddle with 70's wisdoms. And since it's allocated on glibs memory pool, in the end, its faster then char* overall.
Back to top
View user's profile Send private message
Prenj
n00b
n00b


Joined: 20 Nov 2011
Posts: 13

PostPosted: Tue Jan 29, 2013 1:56 pm    Post subject: Reply with quote

BoneKracker wrote:
The instructional material I've read (which is more C++ oriented) frowns on global variables, even disparages and castigates them: evil, evil, evil!

I see them in C code, though. I myself wouldn't have thought of using it, except that when I tried to write signal handling code, it didn't have visibility of this particular thing I wanted to clean up.

Now, I didn't spend much time (like 10 minutes) fiddling with that before giving up (for now). It seems to me there must be a way of passing what's needed to the signal handling function; I just don't know how to do it yet.


I went from textbook C to OOP-like C, where you work with structs and own typedefs as if they were objects, so you pass pointers to structures all the time. If something has to be shared (aka global-like), you have a struct that you pass along. Bit more to handle, but when you have large projects with different people involved, you don't want shady globals. It is also good step towards making code reentrant.
Back to top
View user's profile Send private message
Dr.Willy
Guru
Guru


Joined: 15 Jul 2007
Posts: 323
Location: NRW, Germany

PostPosted: Tue Jan 29, 2013 2:28 pm    Post subject: Reply with quote

BoneKracker wrote:
Ah, you must be a dwm (or similar) user as well. :lol:

Yes ;)

BoneKracker wrote:
(and I'll probably also switch to xcb and borrow heavily from your signal handling, if you don't mind)

In fact I don't mind.

My attorney will contact you shortly wrt licensing fees :lol:

BoneKracker wrote:
Is there any particular reason why you're passing (&buf[len]) as opposed to (buf + len)?

An array name is a pointer variable, so you're forcing it to identify the address of a position within the array. It seems to me it would be more efficient simply to use pointer arithmetic and add the offset to the base pointer (i.e. just pass (buf + len) )?

Cosmetics. I just don't want to wrap my head around pointer arithmetics.
Not sure if that info is still up to date, but buf[1] was equivalent to *(buf+1) so much that you could even do 1[buf]. Argh :lol:
Back to top
View user's profile Send private message
Bones McCracker
Veteran
Veteran


Joined: 14 Mar 2006
Posts: 1563
Location: U.S.A.

PostPosted: Tue Jan 29, 2013 2:59 pm    Post subject: Reply with quote

There may be a good reason for it. Now that I'm trying to replace my strelen(buf) with a "pos" variable (the accumulated length of previous status elements), something's going wrong. It's compiling fine, and its accumulating the lengths accurately, but for some reason now the second and third functions are failing to append anything (which makes me wonder where they are writing it). 8O

So I have to look some more into how this is done properly (using an offset to the base pointer). strlen returns a size_t, but even when I made the return types of the functions size_t and my position variable size_t, I was still just getting the "Load: 0.15" element.

Debugging time, I guess.
_________________
True Liberals are individualists. Democrats, on the other hand, are authoritarian collectivists.
Back to top
View user's profile Send private message
Bones McCracker
Veteran
Veteran


Joined: 14 Mar 2006
Posts: 1563
Location: U.S.A.

PostPosted: Tue Jan 29, 2013 4:18 pm    Post subject: Reply with quote

Hmm... I got it working now. I don't know what I did wrong. Must have been PEBKAC. When I stepped through the changes, function by function, and tested as I went, it all worked.

So now I am using the return values of my load function and ram function (both of which use a snprintf() to create their output), and using pointer arithmetic based on those values in the subsequent calls:
Code:
  while(1) {

    len = append_load(status);
    len += append_ram(status + len);
    append_time(status + len);

    XStoreName(dpy, DefaultRootWindow(dpy), status);
    XSync(dpy, False);

    sleep(1);

  }


I noticed that strftime is slightly different, in that it's return value does not count the terminating null, whereas snprintf does (although this doesn't come into play for me, since I'll always want time last).

Also eliminated the array in the load function.

Another question:

Am I correct in my understanding of the use of 'static' to declare a variable in a non-main function: that it will cause the memory for that variable to be allocated once only and retained across subsequent calls to the function? Based on that understanding, I used it for all the variables in my non-main functions, believing that this will obviate the need for the memory for those variables to be repeatedly allocated and freed each time the function is called. Is that belief correct?
_________________
True Liberals are individualists. Democrats, on the other hand, are authoritarian collectivists.
Back to top
View user's profile Send private message
Bones McCracker
Veteran
Veteran


Joined: 14 Mar 2006
Posts: 1563
Location: U.S.A.

PostPosted: Tue Jan 29, 2013 5:30 pm    Post subject: Reply with quote

How can there be man pages for each of 2,040 distinct xcb functions,
but no man page for xcb_connect or xcb_disconnect?
_________________
True Liberals are individualists. Democrats, on the other hand, are authoritarian collectivists.
Back to top
View user's profile Send private message
Dr.Willy
Guru
Guru


Joined: 15 Jul 2007
Posts: 323
Location: NRW, Germany

PostPosted: Tue Jan 29, 2013 6:00 pm    Post subject: Reply with quote

Yeah one reason for the slow adoption of xcb is definatly the lack of good documentation. :lol:
Back to top
View user's profile Send private message
Bones McCracker
Veteran
Veteran


Joined: 14 Mar 2006
Posts: 1563
Location: U.S.A.

PostPosted: Tue Jan 29, 2013 6:14 pm    Post subject: Reply with quote

Okay, here's the revised version (still might add signal handling). This is now happily humming away on my system, thanks to you guys.

Dr. Willy may want to look at the xcb stuff. I decided just to freeball it, using the man pages, and un-freaking-believably, this worked on the first try (i.e., I did not get a single xcb-related warning or error). I'm quite astounded. Didn't use iccm or aux, and it seems simple enough.

Code:
#include <stdio.h>        // snprintf()
#include <stdlib.h>       // getloadavg()
#include <unistd.h>       // sleep()
#include <time.h>
#include <string.h>
#include <xcb/xcb.h>


static size_t append_load(char *st);
static size_t append_ram(char *st);
static size_t append_time(char *st);


int main() {

  const size_t outsize = 70;
  char status[outsize];
  size_t len;
  xcb_connection_t *c;
  xcb_screen_t *s;

  c = xcb_connect(NULL,NULL);
  s = xcb_setup_roots_iterator(xcb_get_setup(c)).data;

  while(1) {

    len = append_load(status);
    len += append_ram(status + len);
    len += append_time(status + len);

    xcb_change_property(c, XCB_PROP_MODE_REPLACE, s->root,
                        XCB_ATOM_WM_NAME, XCB_ATOM_STRING, 8, len, status);
    xcb_flush(c);

    sleep(1);

  }

  xcb_disconnect(c);
  return 0;

}


// Reads system load (1 min avg) and appends to passed status string.                                   
static size_t append_load(char *st) {

  static const size_t max = 14;
  static double load;

  getloadavg(&load, 1);

  return snprintf(st, max, "  Load: %.2f ", load);

}


// Calculates RAM in use and appends to passed status string.                                           
static size_t append_ram(char *st) {

  static const size_t max = 32;
  static FILE *fp;
  static char label[11];
  static int value;
  static int found;
  static int memstats[6] = {0};
  static int ram_used;
  static int swap_used;

  memset(label, 0, sizeof(label));
  found = 0;

  fp = fopen("/proc/meminfo", "r");

  // search by label because /proc/meminfo entries vary                                                 
  while ( fscanf(fp, "%10s %d %*s", label, &value) != EOF ) {

    if ( strcmp(label, "MemTotal:") == 0 ) {
      memstats[0] = value;
      found ++;
    } else if ( strcmp(label, "MemFree:") == 0 ) {
      memstats[1] = value;
      found ++;
    } else if ( strcmp(label, "Buffers:") == 0 ) {
      memstats[2] = value;
      found++;
    } else if ( strcmp(label, "Cached:") == 0 ) {
      memstats[3] = value;
      found++;
    } else if ( strcmp(label, "SwapTotal:") == 0 ) {
      memstats[4] = value;
      found++;
    } else if ( strcmp(label, "SwapFree:") == 0 ) {
      memstats[5] = value;
      found++;
    }

    if ( found >=6 ) break;

  }

  fclose(fp);

  ram_used = memstats[0] - ( memstats[1] + memstats[2] + memstats[3] );
  swap_used = memstats[4] - memstats[5];

  return snprintf(st, max, "  RAM: %d MiB  Swap: %d MiB ", ram_used/1024, swap_used/1024);

}


// Creates formatted local time and appends to status string.                                           
static size_t append_time(char *st) {

  static const size_t max = 24;
  static time_t now;
  static struct tm ltm;

  time(&now);
  localtime_r(&now, &ltm);
  return strftime(st, max, "  %a %b %-e  %T ", &ltm);

}

_________________
True Liberals are individualists. Democrats, on the other hand, are authoritarian collectivists.
Back to top
View user's profile Send private message
Display posts from previous:   
Reply to topic    Gentoo Forums Forum Index Off the Wall All times are GMT
Goto page Previous  1, 2, 3
Page 3 of 3

 
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum