Re: Parse errors

• Previous message: Floyd L. Davidson: "Re: Parse errors"
• In reply to: Floyd L. Davidson: "Re: Parse errors"
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Date: 28 Jun 2005 14:11:34 -0700

I wish my lecturer explained things to me the way you did, after
altering the macro I was able to successfully compile the client, I do
appreciate all the advice you provided and I will put them into
practice.

Thanks again

Floyd L. Davidson wrote:
> 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: Floyd L. Davidson: "Re: Parse errors"
• In reply to: Floyd L. Davidson: "Re: Parse errors"
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]