Re: Parse errors

• Previous message: Jan Panteltje: "Re: Disk I/O stats from /proc/diskstats"
• In reply to: balzano_1_at_yahoo.com: "Re: Parse errors"
• Next in thread: balzano_1_at_yahoo.com: "Re: Parse errors"
• Reply: balzano_1_at_yahoo.com: "Re: Parse errors"
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Date: Tue, 28 Jun 2005 12:40:16 -0800

balzano_1@yahoo.com wrote:
>The code below is the content of info.h, this is the smallest version i
>can find that works.

Your code, shown in the previous message, will not compile with either
gcc 2 or 3. Note that gcc version 2 follows the 1989 C Standard, while
gcc 3 follows the 1999 C Standard. Which is to say that you can expect
different error messages.

Specificially, this macro causes problems:

   #define SIZE sizeof(struct sockaddr_in);

The semicolon shouldn't be there.

Regardless of that, your code has several problems, and it is probably
true that you are not letting the compiler warn you about some of the
things that it can. The *minimum* warnings should be,

  gcc -O2 -W -Wall

But there are others... Here is what I used to compile your code:

gcc -O2 -ansi -pedantic -Wall -W -Wcast-align -Wcast-qual \
        -Wmissing-prototypes -Wshadow -Wnested-externs \
        -Wstrict-prototypes

Normally I would also use -Waggregate-return.

Otherwise, you are not consistent with respect to style, which
makes it more difficult to read. You are also initializing
pointers to NULL for no reason, casting the return from
malloc(), and using 0 where it might be more readable to use
NULL (which clearly indicates that it is a pointer).

This particular message has a file, info.h, that has *nothing*
in it appropriate to a header file! You should never put anything
into a header that allocates space in the program! Functions and
variables should be defined in source code modules, and *never*
in a header file. The point of a header file is to provide *information*
to a compilation module. That information can be about library modules or
other modules in the program, for example. Hence, if you have a variable
defined in a library, or in another module, you can have a *declaration*
in a header file, telling the module it is included into that the variable
exists.

  int x = 0; /* this goes in a source code file, never in a header */

  extern int x; /* *this* is what goes into a header file! */

Lets look at your "header", but consider it just another source file.
You can either just add it into the other file, or make it info.c and
compile it separately (and then make a valid header file with all the
necessary declarations, prototypes, etc.).

First, you need prototypes for all of the functions defined in this
file:

  void info_ip(char *, char *);
  void fill_by_exec_int(char *, FILE *, int *);
  void fill_by_exec_char(char *, FILE *, char *);

Note that you can use the "extern" qualifier, but there really
isn't much point in it. The same goes for having it in the
function definitions.

>extern void fill_by_exec_int(char *cmd, FILE *fd, int *store_var) {
> fd = popen(cmd, "r");
> if(fscanf(fd, "%d", store_var) != 1){
> printf("error fscanf()"); } return; }

Let's re-write the above to make it readable!

void
fill_by_exec_int(char *cmd, FILE *fp, int *store_var)
{
  fp = popen(cmd, "r");

  if (fscanf(fp, "%d", store_var) != 1) {
    printf("error fscanf()");
  }

  return;
}

There are many valid styles, and I don't mean to say that mine
is the only right way to do it. Do look carefully at the
differences, but *don't* try to define your own style! Instead
find a style that you can live with, and copy it *exactly*. And
as long as you have opinions such as "Hmmm, why can't I change
that, to this?", *DON'T* change it. There will come a day when
you'll *know* why you want to change it. Until then, don't.

Note that I changed your variable "fd" to "fp". Universally
"fd" means a "file descriptor" which is a type int, while "fp"
is a FILE pointer.

Also note that no useful C style that I can think of will do
something like "} return; }" at the end of a line.

One point you might miss, is how operators and functions are
commonly spaced differently. The way I do it above is,

  printf(...)

  for (...)

some people do exactly the opposite,

  printf (...)

  for(...)

Pick one or the other, and religiously write code that way.

>extern info run(void){
> info *inf = NULL;

There is no point in setting these variables equal to NULL, if
they are then immediately set to some other value by code in the
function.

> FILE *fp = NULL;

The one exception was this one, which is never directly set, so
this is okay because it keeps the compiler happy.

> inf = (info *)malloc(sizeof(info));

There is no point in casting the return value from malloc(), and
doing so can cause problems (e.g., if you forget to include the
stdlib.h header file and therefore have no prototype in scope
for malloc(), it will return a type int instead of a void
pointer).

> usrs = (char *)malloc(128);
> if(usrs == 0){

It really isn't any different, but if you use NULL instead of
0 it makes it obvious that his is a pointer context.

> return (*inf);

Another "no real difference" item, but you don't need those
parens. "return *inf;" works just as well.

I compiled, but did not try to run your code. It may or may not
work. But if you fix the macro, it does compile with either gcc
version 2 or 3. However, I also put "#define _GNU_SOURCE" right
at the top of the file too.

-- 
Floyd L. Davidson           <http://web.newsguy.com/floyd_davidson>
Ukpeagvik (Barrow, Alaska)                         floyd@barrow.com

• Previous message: Jan Panteltje: "Re: Disk I/O stats from /proc/diskstats"
• In reply to: balzano_1_at_yahoo.com: "Re: Parse errors"
• Next in thread: balzano_1_at_yahoo.com: "Re: Parse errors"
• Reply: balzano_1_at_yahoo.com: "Re: Parse errors"
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]