Nick Birnie wrote:
Thanks for your input. This is quire correct. May I ask, do you own a
hard copy of the C standard? I am going to buy a copy of a C text soon
as my existing text is very much a textbook. Would I be well advised to
go for Kernighan & Ritchie's circa 1989 "The C Programming Lanaguage" as
opposed to a copy of the C99 standard?
You can get drafts of the C standards for free, one of which is C99 with
all three approved TCs. See
http://clc-wiki.net/wiki/c_standard for
appropriate links.
The C standard is not designed as a programmers reference. Personally I
like K&R2, but there are other reference books people here recommend as
well. A quick search should find them.
Attached updated proc fs parser follows.
thread_status_t
process_state(int pid) {
#define BUFLEN 64
#define LINUM 2
As you are trying to do these with a limited scope, and they are small
integer values, you could use the enum "trick" which automatically
scopes them.
enum { BUFLEN=64, LINUM=2 };
Also, using values from limits.h and sizeof you can calculate the
maximum buffer size at compile time and so not have to worry about
buffer overflow. The number of bits in an int is CHAR_BIT * sizeof(int).
Now, some of those could be padding bits, so the number of value bits
could be smaller, but not larger. 3 bits can represent a number from 0
to 7. So a slight over-estimate (unless there are lots of padding bits)
of the number of decimal digits is (CHAR_BIT * sizeof(int)) / 3. Then
just add on space for the rest of the string (including the nul
termination character) and you have a buffer guaranteed to be large
enough, and probably only a little larger than required.
char buffer[BUFLEN], state = -1;
char could be unsigned! Personally I would use
state = 0
since that is also not a "valid" state for your usage, and sidesteps the
issuf of char being unsigned nicely. Note that gcc has an option to
select this behaviour, so on Linux you cannot guarantee that char is
signed when someone else compiles your code.
FILE *proc_fs_p;
int coln_count = 1, tmp = 0;
if (snprintf(buffer, BUFLEN, "/proc/%d/status", pid) >= BUFLEN)
fatal_error("buffer overflow detected");
Having created a buffer definitely large enough you can then use sprintf
with safety and additional portability (to older Linux before glibc had
sprintf) or be completely paranoid and leave in the check.
proc_fs_p = fopen(buffer , "r");
if (!proc_fs_p)
syserr("/proc open failed");puts("state found");
#define CONSUME (fgets(buffer, BUFLEN, proc_fs_p))
while (CONSUME != NULL){
Why the CONSUME macro when you only use it the once? In my opinion it
would be clearer to not have it and simply write
while (fgets(buffer, BUFLEN, proc_fs_p) != NULL)
or even, if you prefer
while (fgets(buffer, BUFLEN, proc_fs_p))
if (++tmp == LINUM) {
for (int i = 0; i < BUFLEN; i++){
If you are paranoid (which is not unreasonable, and you are allowing for
other failures) you should also terminate the loop on buffer
==0
if (buffer == ':')
coln_count--;
else if (!coln_count && !isblank(buffer)){
state = buffer;
break;
}
}
break;
}
}
fclose(proc_fs_p);
switch (state){
case 'R': //fall through
case 'W': //fall through
case 'S': return TS_RUNNING;
case 'T': return TS_SLEEP;
case 'Z': return TS_FINNISHED;
default:
info("unable to determine process state");
return TS_ERR;
}
return TS_ERR; // unreachable
If you are going to have this return, then I would say get rid of the
default case and have it's code after the switch. Then you don't have
unreachable code!
#undef BUFLEN
#undef LINUM
#undef CONSUME
Using enum as I suggested (which some consider to be abusing it) and not
using the CONSUME macro would allow you to get rid of all of these #undefs.