Re: [LONG] Re: Request for comment on my tiny learning project: dfighterdb (mytree)

dfighter <dfighter@xxxxxxxxxxxx> writes:
[...]
Thank you Richard for you observations. Since you told me it's ok to
post larger codebases too, I'm posting it now.


------------------------------SOURCECODE--------------------------------------

ui.h:

#ifndef __DBUI__
#define __DBUI__
[...]
#endif



tree.h:

#ifndef __MYTREE_H__
#define __MYTREE_H__
[...]
#define FILECORRUPT -1

typedef struct __mytreeinfo__ {
unsigned char numfields;
unsigned long numrecords;
unsigned char *afieldtypes;
char **afieldnames;
}treeinfo_t;
[...]

Just a couple of very quick observations (I haven't had time to look
over the whole thing).

Identifiers starting with two underscores are reserved to the
implementation, which means you're not allowed to use them yourself
for any purpose. It's best to avoid all identifiers starting with
underscores.

For include guards, it's often useful to have a consistent naming
convention based on the header name. Since identifiers starting with
'E' are also reserved (for error codes in <errno.h>), starting the
identifier with the name of the header can cause problems. One
convention I've seen is for the header guard macro for "foo.h" to be
"H_FOO".

You have both a tag (__mytreeinfo__, which you need to change) and a
typedef name (treeinfo_t) for your struct type. There's no real need
to use different identifiers. You could declare

typedef struct treeinfo_t {
/* ... */
} treeinfo_t;

(But I think the "_t" suffix is reserved by POSIX.)

In this case, you don't even need the struct tag, since you never
refer to it:

typedef struct {
/* ... */
} treeinfo_t;

Finally, your macro definition:
#define FILECORRUPT -1
should be:
#define FILECORRUPT (-1)

Remember that the expansion of the macro is not the value -1, it's the
token sequence "-", "1". You want the "-" to be a unary minus
operator, but it might not be interpreted that way in all contexts.
For example, with your definition, this:

int x = 42;
x = x FILECORRUPT;

would be legal, and would set x to 41. With parentheses in the
definition, you'd get the syntax error message that you want. (There
may be more plausible examples.)

--
Keith Thompson (The_Other_Keith) kst-u@xxxxxxx <http://www.ghoti.net/~kst>
Nokia
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
.

Relevant Pages