Request for comment on my tiny learning project: dfighterdb (mytree)

D

dfighter

Dear everyone!
dfighterdb (I started it as mytree), is a tiny and simple console based
"database" application I'm writing as a learning project.
It's purpose is to teach me some generic dynamic data structures, and
general design of "larger" projects (Basically a binary tree and a
linear list with 256 generic fields).
It's not complete yet, however it is complete enough to present an
insight into my "coding style", to show how well (or badly) I can use
the language, how portable the code is, general design of the project, etc.
So I'd like to ask you all if you have the time to review it and comment
on it.

zipped source code + win32 binary:
http://www.freeweb.hu/dfighter/dfighterdb.zip

mirror: http://jesterpm.net/filebin2/1429525413/dfighterdb.zip


Thanks a bunch in advance.

dfighter
 
D

dfighter

Han said:
If it's tiny, you may as well post the source code to the group.
This has at least two advantages:

(1) Some people are unable or unwilling to check external links
or to unzip archives, so you've already lost their help.

(2) WWW links die all the time, but your posts will have a relatively
permanent existence in various Usenet archives. There's a chance
someone who's writing a similar application could happen upon this
thread years from now and want to know what everyone in the thread
is referring to.



Yours,
Han from China
Unfortunately it's not that tiny. It's only relatively tiny (~900 lines).
(1) That's very unfortunate.
(2) You have a point, however I can always choose not to delete it from
the website ;)
 
C

CBFalconer

dfighter said:
Unfortunately it's not that tiny. It's only relatively tiny (~900
lines).
(1) That's very unfortunate.
(2) You have a point, however I can always choose not to delete it
from the website ;)

FYI Han is a troll. He has even announced that his objective is to
destroy c.l.c. Best plonked and ignored.
 
L

luserXtrog

Unfortunately it's not that tiny. It's only relatively tiny (~900 lines).
(1) That's very unfortunate.
(2) You have a point, however I can always choose not to delete it from
the website ;)

*always*?
You do realize how long always is?!

I, for one, am too lazy to go get it. But if you bring it over here...

I have posted long sources here. There was a solitary request that
a link be posted instead. But that one request was withheld from the
archive. Therefore, as the record shows, no one ever complained
about its length (its content, however, had some issues).

You could add something to the subject, perhaps: [Warning ~900 lines!]
to give the bandwidth-poor an opt-out.

Even a link to a directory of sources files would be better (ie.
more indulgent of the lazy) than a zip file.
 
D

dfighter

Richard said:
dfighter said:


Well, well, well. Even a stopped clock is right twice a day.


That's pretty tiny - okay, quite a large tiny, but nevertheless well
within Usenet limits (put LONG in the subject line as a warning to
the modem users amongst us, and you're covered). This isn't IRC.


Sometimes that isn't within our control. When I moved ISP, I left a
number of pages behind that I can no longer maintain. Worse still,
they're still there, so few people realise that the pages are no
longer maintained.

I hope this won't happen again (not because I won't move ISPs again,
but because I now do my own hosting, with Web forwarding).

ANYWAY - I did actually look through your code. I didn't have time
for a full review, I'm afraid, but before I realised this I did
spot a few minor conformance issues (the one I remember being
leading underscores on identifiers) and a design consideration (as
a matter of robustness you might want to take T** in your
destructors, rather than T*, so that you can set *p to NULL when
you're done).
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__

#define TABLE_OK 0
#define TABLE_ALREADY_EXISTS 1
#define TABLE_ERROR -1

void ui_mainmenu(void);
#endif



tree.h:

#ifndef __MYTREE_H__
#define __MYTREE_H__

#define MAXFIELDNAMELENGTH 100
#define SIGLENGTH 6
#define SIG "DBTBL"


#define FILECORRUPT -1

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

typedef struct __mytreenode__ {
struct __mytreenode__ *parent;
struct __mytreenode__ *left;
struct __mytreenode__ *right;
void **field;
}treenode_t;

typedef struct __mytreefilehdr__{
char signature[SIGLENGTH];
unsigned long numrecords;
unsigned char numfields;
unsigned char *afieldtypes;
char **afieldnames;
}treefilehdr_t;


/*
*
* Tree handling functions
*
**/

void treeinfo_init(treeinfo_t *info, unsigned char numfields, unsigned
char *afieldtypes);
void treeinfo_setfieldname(treeinfo_t *info, unsigned char fid, char *s);
void treeinfo_destroy(treeinfo_t *info);

void treenode_destroy(treenode_t *node, unsigned char numfields);
treenode_t* treenode_allocate(unsigned char numfields, unsigned char
*afieldtypes);

void tree_destroy(treenode_t *root, unsigned char numfields);
treenode_t* tree_addnode_nr(treenode_t *root, treeinfo_t *info, void**
fields);


void tree_dump(treenode_t *root, treeinfo_t *info);

unsigned long tree_serialize(treenode_t *root, treeinfo_t *info, FILE *fp);
unsigned long tree_writeheader(treeinfo_t *info, FILE *fp);
unsigned long tree_readhdr(treeinfo_t **info, FILE *fp, int *error);
unsigned long tree_deserialize(treenode_t **root, treeinfo_t *info, FILE
*fp);

#endif


table.h:

#ifndef __TABLE_H__
#define __TABLE_H__

int table_exists(void);
int table_create(unsigned char numfields, unsigned char *fieldtypes);
void table_destroy(void);
void table_setfieldname(unsigned char fid, char *s);
char* table_fieldname(int fid);
unsigned char table_numfields(void);
unsigned char table_fieldtype(int fid);
int table_addrecord(void **v);
void table_dump(void);
unsigned char* table_afieldtypes(void);
void* table_tableroot(void);
void table_save(void);
void table_load(void);

#endif


search.h:

#ifndef __SEARCH_H__
#define __SEARCH_H__

#define RESET 1
#define SEARCH 0
void search_search(unsigned char keyid, void *value, unsigned char
orderbyfield);
searchres_t* search_result_node(unsigned char reset);
void search_dropresults(void);
#endif


list.h:

#ifndef __LIST_H__
#define __LIST_H__

typedef struct list{
struct list* next;
unsigned char dummy;
void **field;
}list_t;


void listnode_destroy(list_t *node, unsigned char numfields);
void list_destroy(list_t *root, int numfields);
list_t* listnode_allocate(unsigned char dummy, unsigned char numfields,
unsigned char *afieldtypes);
list_t* list_addnode_ordered(list_t *root, void** values, unsigned char*
afieldtypes, unsigned char numfields, unsigned char orderbyfieldno);

#endif


common.h:

#ifndef __COMMON_H__
#define __COMMON_H__

typedef struct search_result{
void **v;
unsigned char fieldnum;
unsigned char* afieldtypes;
}searchres_t;

enum fieldtypes { CHAR,INT,DOUBLE };

void* field_allocate(unsigned char type);
int field_compare(void* val1, void* val2, unsigned char type);
void field_setvalue(void *field, void *value, unsigned char type);

#endif




ui.c:

#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#include <string.h>
#include "ui.h"
#include "common.h"
#include "table.h"
#include "search.h"

#define MAXINPUTLINELENGTH 255
#define MAXFIELDS 255
#define MAXFIELDNAMELENGTH 100
#define QUITKEY 'q'
#define BACKKEY 'b'

static char *ui_menu_main_szitem[] = {
"Table",
"Quit",
NULL
};

static char *ui_menu_table_szitems[] = {
"Create",
"Destroy",
"Load",
"Save",
"Query",
"Dump",
"Back",
NULL
};

static char *ui_menu_table_query[] = {
"New record",
"Search",
"Back",
NULL
};

static char ui_menu_kmain[] = {
't',
'q',
0
};

static char ui_menu_ktable[] = {
'c',
'd',
'l',
's',
'q',
'p',
'b',
0
};

static char ui_menu_ktablequery[] = {
'n',
's',
'b',
0
};

static char *sztypes[] = {
"CHAR",
"INT",
"DOUBLE"
};


static char *header = "dfighterdb UI";
static char *separator = "-------------";

static void ui_trim(char *s){
int i = 0;
for(i = 0; s != '\0'; i++)
;

while(i > 0 && !isalnum(s))
i--;

s[i+1] = '\0';
}

static int ui_getmenuchoice(void){
char line[MAXINPUTLINELENGTH];
fgets(line,MAXINPUTLINELENGTH,stdin);

return line[0];
}

static int ui_getINT(int *error){
char line[MAXINPUTLINELENGTH];
int value = 0;
fgets(line,MAXINPUTLINELENGTH,stdin);

if(!isdigit(line[0]))
*error = -1;
else value = (int)atof(line);

return value;

}

static double ui_getDOUBLE(int *error){
char line[MAXINPUTLINELENGTH];
double value = 0.0;
fgets(line,MAXINPUTLINELENGTH,stdin);

if(!isdigit(line[0]) && line[0] != '+' && line[0] != '-')
*error = -1;
else value = atof(line);

return value;
}

static int ui_getSTRING(char *s){
fgets(s,MAXINPUTLINELENGTH,stdin);
ui_trim(s);
if(!isalpha(s[0]))
return -1;
else return 0;
}


static void ui_tablemenu_create(void){
int status = 0;
int numfields = 0;
int error = 0;
int i = 0;
int type = 0;
unsigned char fieldtypes[MAXFIELDS];
char fieldnames[MAXFIELDS][MAXFIELDNAMELENGTH];
char name[MAXINPUTLINELENGTH];

status = table_exists();
if(status == TABLE_ALREADY_EXISTS){
printf("table already exists\n");
return;
}

printf("create new table\n");
printf("%s\n",separator);
error = 0;
do{
printf("number of fields:");
numfields = ui_getINT(&error);
if(error != 0)
printf("invalid input.\n");
}while(error != 0);

if(numfields > MAXFIELDS)
numfields = MAXFIELDS;

printf("possible field types:\n\n");

for(i = 0; i < 3; i++)
printf("%s - %d\n",sztypes, i);

putchar('\n');

for(i = 0; i < numfields; i++){
error = 0;

do{
printf("name of field%d:",i);
error = ui_getSTRING(name);
}while(error != 0);

/* table_setfieldname(i,name); */
strncpy(fieldnames,name,MAXFIELDNAMELENGTH);

}

for(i = 0; i < numfields; i++){
error = 0;

do{
printf("type of \"%s\":",fieldnames);
type = ui_getINT(&error);
if(error != 0 || type > 2)
printf("invalid input.\n");
}while(error != 0);

fieldtypes = (unsigned char)type;
}

printf("Creating table...");
if(table_create((unsigned char)numfields,fieldtypes) == TABLE_OK){
for(i = 0; i < numfields; i++)
table_setfieldname((unsigned char)i,fieldnames);
printf("done.\n");
}
else printf("error.\n");

}

static void ui_tablemenu_destroy(void){
if(table_exists() != TABLE_ALREADY_EXISTS)
printf("Nothing to destroy.");
else{
printf("destroying table..");
table_destroy();
}
}


void ui_tablemenu_query_new(void){
unsigned char numfields = table_numfields();
int i = 0;
void **v = NULL;
unsigned char type = 0;
char *fieldname = NULL;
int error = 0;
char cval = 0;
int ival = 0;
double dval = 0;

v = malloc(numfields*sizeof(void*));
if(v == NULL){
printf("ERROR: not enough memory\n");
return;
}

for(i = 0; i < numfields; i++){
type = table_fieldtype(i);
v = NULL;

switch(type){
case CHAR: v = malloc(sizeof(char)); break;
case INT: v = malloc(sizeof(int)); break;
case DOUBLE: v = malloc(sizeof(double)); break;
}

if(v == NULL){
i--;
while(i >= 0)
free(v);
free(v);
printf("ERROR: not enough memory.\n");
return;
}
}

/* getting and setting the values */
for(i = 0; i < numfields; i++){
type = table_fieldtype(i);
fieldname = table_fieldname(i);

printf("%s (%s):",fieldname,sztypes[type]);

error = 0;

do{

switch(type){
case CHAR: cval = (unsigned char)ui_getmenuchoice();
field_setvalue(v,&cval,type); break;
case INT: ival = ui_getINT(&error); field_setvalue(v,&ival,type);
break;
case DOUBLE: dval = ui_getDOUBLE(&error);
field_setvalue(v,&dval,type); break;
}
}while(error != 0);
}

error = table_addrecord(v);
if(error == TABLE_OK)
printf("Record added successfully.\n");
else printf("Record couldn't be added.\n");

/* freeing the array */
for(i = 0; i < numfields; i++){
free(v);
v = NULL;
}
free(v);
v = NULL;
}

void ui_tablemenu_query_search(void){
unsigned char i = 0;
unsigned char j = 0;
unsigned char numfields = table_numfields();
char *name = NULL;
int error = 0;
unsigned char keyid = 0;
char cval = 0;
int ival = 0;
double dval = 0.0;
searchres_t *sr = NULL;
void *val = NULL;
int orderkeyid = 0;

search_dropresults();

printf("Search:\n");
printf("%s\n",separator);
printf("Possible search keys:\n");
for(i = 0; i < numfields; i++){
name = table_fieldname(i);
printf("%d.) %s\n",i,name);
}
putchar('\n');

error = 0;
do{
printf("Search key id:");
keyid = (unsigned char)ui_getINT(&error);
}while(error != 0);

error = 0;
do{
printf("Value:");
switch(table_fieldtype(keyid)){
case CHAR: cval = (char)ui_getmenuchoice(); val = (void*)&cval; break;
case INT: ival = ui_getINT(&error); val = (void*)&ival; break;
case DOUBLE: dval = ui_getDOUBLE(&error); val = (void*)&dval; break;
}

}while(error != 0);

printf("Fields:\n");
for(i = 0; i < numfields; i++){
name = table_fieldname(i);
printf("%d.) %s\n",i,name);
}
putchar('\n');

do{
printf("Order by:");
orderkeyid = ui_getINT(&error);
}while(error != 0 && orderkeyid < 0);

printf("Searching... ");

search_search(keyid,val,(unsigned char)orderkeyid);
search_result_node(1);
printf("Done.\n");
printf("Results:\n");

for(sr = search_result_node(SEARCH); sr != NULL; sr =
search_result_node(SEARCH)){
for(j = 0; j < sr->fieldnum; j++){
printf("%s: ",table_fieldname(j));

switch(sr->afieldtypes[j]){
case CHAR: printf("%c",*(char*)sr->v[j]); break;
case INT: printf("%d",*(int*)sr->v[j]); break;
case DOUBLE: printf("%lf",*(double*)sr->v[j]); break;
}
putchar('\n');
}
putchar('\n');
}

}

void ui_tablemenu_query(void){
int i = 0;
int ch = 0;

if(table_exists() == TABLE_ERROR){
printf("nothing to query.\n\n");
return;
}

do{
printf("query table\n");
printf("%s\n",separator);

for(i = 0; ui_menu_ktablequery != 0; i++)
printf("%c.) %s\n",ui_menu_ktablequery,ui_menu_table_query);

putchar('\n');
putchar('#');
ch = ui_getmenuchoice();


switch(ch){
case 'n': ui_tablemenu_query_new(); break;
case 's': ui_tablemenu_query_search(); break;
}
}while(ch != BACKKEY);

}

static void ui_tablemenu(void){
int i = 0;
int ch = 0;

do{
printf("%s\n","table");
printf("%s\n",separator);

for(i = 0; ui_menu_table_szitems; i++)
printf("%c.) %s\n",ui_menu_ktable,ui_menu_table_szitems);

putchar('\n');
putchar('#');

ch = ui_getmenuchoice();
ch = tolower(ch);

switch(ch){
case 'c': ui_tablemenu_create(); break;
case 'd': ui_tablemenu_destroy(); break;
case 'q': ui_tablemenu_query(); break;
case 's': table_save(); break;
case 'l': table_load(); break;
case 'p': table_dump(); break;
}

}while(ch != EOF && ch != BACKKEY );

}

void ui_mainmenu(void){
int i = 0;
int ch = 0;


do{
printf("%s\n",header);
printf("%s\n",separator);
for(i = 0; i < 2; i++)
printf("%c.) %s\n",ui_menu_kmain,ui_menu_main_szitem);
printf("\n#");



ch = ui_getmenuchoice();
ch = tolower(ch);

switch(ch){
case 't': ui_tablemenu(); break;
}

printf("\n\n");

}while(ch != EOF && ch != QUITKEY);

search_dropresults();
if(table_exists() == TABLE_ALREADY_EXISTS)
table_destroy();
}


tree.c:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "tree.h"
#include "common.h"

/* TreeInfo structure manipulating functions */

void treeinfo_init(treeinfo_t *info, unsigned char numfields, unsigned
char *afieldtypes){
int i = 0;

info->numfields = numfields;
info->afieldtypes = malloc(numfields*sizeof(unsigned char));
info->afieldnames = malloc(numfields*sizeof(char*));
info->numrecords = 0;

for(i = 0; i < numfields; i++)
info->afieldtypes = afieldtypes;

for(i = 0; i < numfields; i++)
info->afieldnames = malloc(MAXFIELDNAMELENGTH*sizeof(char));

}

void treeinfo_setfieldname(treeinfo_t *info, unsigned char fid, char *s){
strncpy(info->afieldnames[fid],s,MAXFIELDNAMELENGTH);
}

void treeinfo_destroy(treeinfo_t *info){
int i = 0;

if(info->afieldtypes != NULL)
free(info->afieldtypes);

for(i = 0; i < info->numfields; i++)
if(info->afieldnames != NULL && info->afieldnames != NULL)
free(info->afieldnames);

if(info->afieldnames != NULL)
free(info->afieldnames);
}

/* Treenode manipulating functions */

void treenode_destroy(treenode_t *node, unsigned char numfields){
int i = 0;

for(i = 0; i < numfields; i++)
if(node->field != NULL)
free(node->field);
free(node->field);
free(node);
}

treenode_t* treenode_allocate(unsigned char numfields, unsigned char
*afieldtypes){
treenode_t *newnode = NULL;
int i = 0;
int error = 0;

newnode = malloc(sizeof(treenode_t));
if(newnode == NULL)
return NULL;

newnode->field = NULL;
newnode->field = malloc(numfields*sizeof(void*));
if(newnode->field == NULL){
free(newnode);
return NULL;
}

for(i = 0; i < numfields; i++){
newnode->field = NULL;
newnode->field = field_allocate(afieldtypes);
}

for(i = 0; i < numfields; i++){
if(newnode->field == NULL){
error = 1;
break;
}
}

if(error == 1){
treenode_destroy(newnode,numfields);
newnode = NULL;
}

return newnode;
}


void tree_destroy(treenode_t *root, unsigned char numfields){
if(root == NULL)
return;

tree_destroy(root->left,numfields);
tree_destroy(root->right,numfields);
treenode_destroy(root,numfields);
}

/* for diagnostic purposes only, will have to be removed/disabled later */
void tree_dump(treenode_t *root, treeinfo_t *info){
int i = 0;

if(root == NULL)
return;

tree_dump(root->left,info);

for(i = 0; i < info->numfields; i++){
switch(info->afieldtypes){
case CHAR: printf("%c ",*(char*)root->field); break;
case INT: printf("%d ",*(int*)root->field); break;
case DOUBLE: printf("%lf ",*(double*)root->field); break;
}
putchar('\n');
}
putchar('\n');

tree_dump(root->right,info);
}

/* Adding the new node non-recursively */
treenode_t* tree_addnode_nr(treenode_t *root, treeinfo_t *info, void**
fields){
treenode_t *newnode = NULL;
treenode_t *current = NULL;
treenode_t *prev = NULL;
int i = 0;

newnode = treenode_allocate(info->numfields,info->afieldtypes);
if(newnode == NULL)
return root;
newnode->left = NULL;
newnode->right = NULL;

for(i = 0; i < info->numfields; i++)
field_setvalue(newnode->field,fields,info->afieldtypes);

if(root == NULL){
root = newnode;
}else{
current = prev = root;

while(current != NULL)
if(field_compare(fields[0],current->field[0],info->afieldtypes[0]) >= 0){
prev = current;
current = current->right;
}else{
prev = current;
current = current->left;
}

if(field_compare(fields[0],prev->field[0],info->afieldtypes[0]) >= 0){
prev->right = newnode;
newnode->parent = prev;
}else{
prev->left = newnode;
newnode->parent = prev;
}


}

info->numrecords += 1;

return root;
}

unsigned long tree_writeheader(treeinfo_t *info, FILE *fp){
unsigned long written = 0;
int i = 0;

written += fwrite( "DBTBL", SIGLENGTH*sizeof(char), 1, fp);
written += fwrite( &info->numrecords, sizeof(unsigned long), 1, fp);
written += fwrite( &info->numfields, sizeof(unsigned char), 1, fp);
written += fwrite( info->afieldtypes, info->numfields * sizeof(unsigned
char), 1, fp);

for(i = 0; i < info->numfields; i++){
written += fwrite( info->afieldnames, MAXFIELDNAMELENGTH *
sizeof(char), 1, fp);
}


return written;
}

unsigned long tree_readhdr(treeinfo_t **info, FILE *fp, int *error){
unsigned long read = 0;
int i = 0;
char name[MAXFIELDNAMELENGTH];
char sig[SIGLENGTH];
unsigned long numrecords = 0;
unsigned char numfields = 0;
unsigned char *afieldtypes = NULL;

if(info == NULL || *info != NULL)
return 0;

read += fread(sig, SIGLENGTH*sizeof(char), 1, fp);
if(strcmp(sig, SIG) != 0){
*error = FILECORRUPT;
return 0;
}

read += fread(&numrecords, sizeof(unsigned long), 1, fp);
if(&numrecords == 0){
*error = FILECORRUPT;
return 0;
}

read += fread(&numfields, sizeof(unsigned char), 1, fp);
if(&numfields == 0){
*error = FILECORRUPT;
return 0;
}

afieldtypes = malloc(numfields * sizeof(unsigned char));
for(i = 0; i < numfields; i++)
afieldtypes = 0;

*info = malloc(sizeof(treeinfo_t));

treeinfo_init(*info,numfields,afieldtypes);
free(afieldtypes);
afieldtypes = NULL;
(*info)->numfields = numfields;
(*info)->numrecords = numrecords;

read += fread((*info)->afieldtypes, (*info)->numfields *
sizeof(unsigned char), 1, fp);


for(i = 0; i < (*info)->numfields; i++){
read += fread(name, MAXFIELDNAMELENGTH * sizeof(char), 1, fp);
strncpy((*info)->afieldnames, name, MAXFIELDNAMELENGTH);
}

return read;
}

unsigned long tree_serialize(treenode_t *root, treeinfo_t *info, FILE *fp){
unsigned long written = 0;
int i = 0;

if(root == NULL || info == NULL)
return 0;

written += tree_serialize(root->left,info,fp);
written += tree_serialize(root->right,info,fp);

for(i = 0; i < info->numfields; i++){
switch(info->afieldtypes){
case CHAR: fwrite(root->field, sizeof(char), 1, fp); break;
case INT: fwrite(root->field, sizeof(int), 1, fp); break;
case DOUBLE: fwrite(root->field, sizeof(double), 1, fp); break;
}
}

written += 1;

return written;
}

unsigned long tree_deserialize(treenode_t **root, treeinfo_t *info, FILE
*fp){
unsigned long read = 0;
unsigned long i = 0;
unsigned long numrecords = 0;
int j = 0;
void **v = NULL;

if(info == NULL || info->numrecords == 0)
return 0;

numrecords = info->numrecords;
info->numrecords = 0;

v = malloc(info->numfields * sizeof(void*));
if(v == NULL){
printf("ERROR: not enough memory.\n");
return 0;
}

for(i = 0; i < info->numfields; i++){
v = field_allocate(info->afieldtypes);
}

for(i = 0; i < numrecords; i++){
for(j = 0; j < info->numfields; j++){
switch(info->afieldtypes[j]){
case CHAR: fread(v[j], sizeof(char), 1, fp); break;
case INT: fread(v[j], sizeof(int), 1, fp); break;
case DOUBLE: fread(v[j], sizeof(double), 1, fp); break;
}
}

*root = tree_addnode_nr(*root, info, v);
read += 1;
}

return read;

}


table.c:

#include <stdio.h>
#include <stdlib.h>
#include "tree.h"
#include "list.h"
#include "common.h"
#include "ui.h"


static treeinfo_t *tableinfo = NULL;
static treenode_t *tableroot = NULL;

void* table_tableroot(void){
return tableroot;
}

int table_create(unsigned char numfields, unsigned char *fieldtypes){
tableinfo = malloc(sizeof(treeinfo_t));
if(tableinfo == NULL)
return TABLE_ERROR;

treeinfo_init(tableinfo,numfields,fieldtypes);
return TABLE_OK;
}

void table_setfieldname(unsigned char fid, char *s){
treeinfo_setfieldname(tableinfo,fid,s);
}

char* table_fieldname(int fid){
return tableinfo->afieldnames[fid];
}

unsigned char table_numfields(void){
return tableinfo->numfields;
}

unsigned char table_fieldtype(int fid){
return tableinfo->afieldtypes[fid];
}

unsigned char* table_afieldtypes(void){
return tableinfo->afieldtypes;
}

void table_destroy(void){
if(tableinfo == NULL)
return;
if(tableroot != NULL)
tree_destroy(tableroot,tableinfo->numfields);

treeinfo_destroy(tableinfo);
free(tableinfo);

tableinfo = NULL;
tableroot = NULL;
}

int table_exists(void){
if(tableinfo != NULL)
return TABLE_ALREADY_EXISTS;
else return TABLE_ERROR;
}


int table_addrecord(void **v){
treenode_t *tp = NULL;

tp = tree_addnode_nr(tableroot,tableinfo,v);
if(tp != NULL){
tableroot = tp;
return TABLE_OK;
}else return TABLE_ERROR;
}


void table_dump(void){
printf("Dumping records:\n\n");
tree_dump(tableroot,tableinfo);
}

void table_save(void){
FILE *fp = NULL;
unsigned long written = 0;

if(tableinfo == NULL || tableroot == NULL || tableinfo->numrecords == 0){
printf("Nothing to save.\n");
return;
}

printf("Saving table... ");
fp = fopen("table.dat","wb");
if(fp == NULL){
printf("\nERROR: cannot open file.\n");
return;
}

written = tree_writeheader(tableinfo,fp);
if(written == 0){
printf("ERROR: cannot write to file.\n");
fclose(fp);
return;
}
written = tree_serialize(tableroot,tableinfo,fp);
if(written != tableinfo->numrecords){
printf("ERROR: Couldn't save 1 or more records.\n");
fclose(fp);
return;
}

printf("done.\n");
fclose(fp);
}

void table_load(void){
int errval = 0;
FILE *fp = NULL;

if(tableinfo != NULL || tableroot != NULL){
printf("Table already loaded.\n");
return;
}

printf("Loading table... ");

fp = fopen("table.dat","rb");

if(fp == NULL){
printf("\nERROR: cannot open file.\n");
return;
}

tree_readhdr(&tableinfo,fp,&errval);
tree_deserialize(&tableroot,tableinfo,fp);

printf("done.\n");

fclose(fp);
}


search.c:

#include <stdio.h>
#include "list.h"
#include "tree.h"
#include "table.h"
#include "common.h"

typedef struct searchparam{
treenode_t *root;
unsigned char keyid;
void* value;
unsigned char orderby;
unsigned char numfields;
unsigned char *fieldtypes;
}search_t;

static unsigned long numresults = 0;
static list_t *results = NULL;
static search_t search_struct;
static searchres_t searchres;

static void search_buildresultlist(treenode_t *root, search_t *searchs){
if(root == NULL)
return;

search_buildresultlist(root->left,searchs);
search_buildresultlist(root->right,searchs);

if(field_compare(searchs->value,
root->field[searchs->keyid],searchs->fieldtypes[searchs->keyid]) == 0)
results =
list_addnode_ordered(results,root->field,searchs->fieldtypes,
searchs->numfields, searchs->orderby);
}

void search_search(unsigned char keyid, void *value, unsigned char
orderbyfield){

search_struct.root = table_tableroot();
search_struct.keyid = keyid;
search_struct.value = value;
search_struct.orderby = orderbyfield;
search_struct.numfields = table_numfields();
search_struct.fieldtypes = table_afieldtypes();

search_buildresultlist(search_struct.root,&search_struct);
}

void search_dropresults(void){
list_destroy(results,search_struct.numfields);
results = NULL;
}

searchres_t* search_result_node(unsigned char reset){
static list_t *currnode = NULL;

if(results == NULL)
return NULL;

if(reset == 1){
currnode = results;
return NULL;
}

currnode = currnode->next;

if(currnode == NULL || currnode->dummy == 1){
currnode = NULL;
return NULL;
}

searchres.v = currnode->field;
searchres.afieldtypes = search_struct.fieldtypes;
searchres.fieldnum = search_struct.numfields;

return &searchres;
}



main.c:

#include <stdio.h>
#include "ui.h"

void list_test(void);

int main(void){
ui_mainmenu();

return 0;
}


list.c:

#include <stdio.h>
#include <stdlib.h>
#include "common.h"
#include "list.h"

#define DUMMY 1
#define NODE 0


void listnode_destroy(list_t *node, unsigned char numfields){
int i = 0;

if(node->dummy == 0){
for(i = 0; i < numfields; i++)
free(node->field);
free(node->field);
}
free(node);
}

list_t* listnode_allocate(unsigned char dummy, unsigned char numfields,
unsigned char *afieldtypes){
list_t *newnode = NULL;
int i = 0;
int error = 0;

newnode = malloc(sizeof(list_t));
if(newnode == NULL)
return NULL;

newnode->next = NULL;

if(dummy == 0){
newnode->field = malloc(numfields*sizeof(void*));
if(newnode->field == NULL){
free(newnode);
return NULL;
}

for(i = 0; i < numfields; i++){
newnode->field = field_allocate(afieldtypes);
}

for(i = 0; i < numfields; i++){
if(newnode->field == NULL){
error = 1;
break;
}
}

if(error == 1){
listnode_destroy(newnode,numfields);
newnode = NULL;
}
}

newnode->dummy = dummy;

return newnode;
}

list_t* list_addnode_ordered(list_t *root, void** values, unsigned char*
afieldtypes, unsigned char numfields, unsigned char orderbyfieldno){
list_t *newnode = NULL;
list_t *dummy1 = NULL;
list_t *dummy2 = NULL;
list_t *current = NULL;
list_t *prev = NULL;
int i = 0;

newnode = listnode_allocate(NODE,numfields,afieldtypes);
if(newnode == NULL)
return root;

for(i = 0; i < numfields; i++)
field_setvalue(newnode->field,values,afieldtypes);

if(root == NULL){
dummy1 = listnode_allocate(DUMMY,0,NULL);
dummy2 = listnode_allocate(DUMMY,0,NULL);
if(dummy1 == NULL || dummy2 == NULL)
return root;

root = dummy1;
dummy1->next = newnode;
newnode->next = dummy2;
}else{
current = root->next;
prev = root;

while(current->dummy == 0 &&
field_compare(values[orderbyfieldno],current->field[orderbyfieldno],afieldtypes[orderbyfieldno])
prev = current;
current = current->next;
}

prev->next = newnode;
newnode->next = current;
}

return root;
}

void list_destroy(list_t *root, int numfields){
list_t *current = NULL;
list_t *prev = NULL;

if(root == NULL)
return;

current = prev = root;

while(current != NULL){
prev = current;
current = current->next;
listnode_destroy(prev,(unsigned char)numfields);
}

}

void list_test(void){
list_t *mylist = NULL;
list_t *current = NULL;
void **v = NULL;
unsigned char *a = NULL;
unsigned char numfields = 3;
int i = 0;
int j = 0;

printf("testing the list engine....\n");

a = malloc(3*sizeof(unsigned char));
a[0] = DOUBLE;
a[1] = INT;
a[2] = CHAR;

v = malloc(numfields*sizeof(void*));

for(i = 0; i < numfields; i++){
switch(a){
case CHAR: v = malloc(sizeof(char)); break;
case INT: v = malloc(sizeof(int)); break;
case DOUBLE: v = malloc(sizeof(double)); break;
}
}

for(i = 0; i < 10; i++){
*(int*)v[1] = i;
*(double*)v[0] = i*1.66;
*(char*)v[2] = 'z'-(char)i;
mylist = list_addnode_ordered(mylist,v,a,numfields,0);
}



current = mylist->next;

for(j = 0; j < 10; j++){

for(i = 0; i < numfields; i++){
if(current->dummy == 0)
switch(a){
case CHAR: printf("%c ",*(char*)current->field); break;
case INT: printf("%d ",*(int*)current->field); break;
case DOUBLE: printf("%lf ",*(double*)current->field); break;
}

}
current = current->next;
putchar('\n');
}

list_destroy(mylist,numfields);


for(i = 0; i < numfields; i++)
free(v);
free(v);
free(a);
}


common.c:

#include <stdio.h>
#include <stdlib.h>
#include "common.h"

void* field_allocate(unsigned char type){
void* fieldptr = NULL;

switch(type){
case CHAR: fieldptr = malloc(sizeof(char)); break;
case INT: fieldptr = malloc(sizeof(int)); break;
case DOUBLE: fieldptr = malloc(sizeof(double)); break;
default: return NULL; break;
}

return fieldptr;
}

int field_compare(void* val1, void* val2, unsigned char type){
int retval = 0;

switch(type){
case CHAR:{
if(*(char*)val1 > *(char*)val2)
retval = 1;
else
if(*(char*)val1 < *(char*)val2)
retval = -1;
}break;

case INT:{
if(*(int*)val1 > *(int*)val2)
retval = 1;
else if(*(int*)val1 < *(int*)val2)
retval = -1;
}break;

case DOUBLE:{
if(*(double*)val1 > *(double*)val2)
retval = 1;
else
if(*(double*)val1 < *(double*)val2)
retval = -1;
}break;
}

return retval;
}

void field_setvalue(void *field, void *value, unsigned char type){
switch(type){
case CHAR: *(char*)field = *(char*)value; break;
case INT: *(int*)field = *(int*)value; break;
case DOUBLE: *(double*)field = *(double*)value; break;
}
}

--------------------------END OF SOURCECODE---------------------------



dfighter
 
K

Keith Thompson

dfighter said:
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.)
 
F

Flash Gordon

dfighter wrote:

Thank you Richard for you observations. Since you told me it's ok to
post larger codebases too, I'm posting it now.

In addition to Keith's comments (which I agree with)...

#define RESET 1
#define SEARCH 0

If, as I suspect, these are two possible values of a single item an enum
would make sense.

static char *ui_menu_main_szitem[] = {
"Table",
"Quit",
NULL
};

Adding a const qualifier on this and the other similar arrays I've
snipped might help trap errors at compile time.

static char *header = "dfighterdb UI";
static char *separator = "-------------";

Again, a const might be useful.
static void ui_trim(char *s){
int i = 0;
for(i = 0; s != '\0'; i++)


You are setting i to 0 on two consecutive lines. Whilst legal, it does
look wrong to me and is likely (I think) to make people stop and wonder.

static int ui_getmenuchoice(void){
char line[MAXINPUTLINELENGTH];
fgets(line,MAXINPUTLINELENGTH,stdin);

return line[0];

This is rather odd. If you want to throw away all characters after the
first on the line it won't work if the line is too long (someone
accidentally put something down on the keyboard). It would be better to
make sure you have read up to EOF or a newline in my opinion.
}

static int ui_getINT(int *error){
char line[MAXINPUTLINELENGTH];
int value = 0;
fgets(line,MAXINPUTLINELENGTH,stdin);

if(!isdigit(line[0]))
*error = -1;
else value = (int)atof(line);

atof is not a good function to use for several reasons. How will you do
error checking with it? Why use a function to decode a floating point
number when what you want is an integer value? strtol and checking the
result is in range of an int (make value a long) and checking for errors
would be better. Even without checking for errors (just range) it would
still be better! Also the cast is not needed and does not help.
return value;

}

static double ui_getDOUBLE(int *error){
char line[MAXINPUTLINELENGTH];
double value = 0.0;
fgets(line,MAXINPUTLINELENGTH,stdin);

if(!isdigit(line[0]) && line[0] != '+' && line[0] != '-')
*error = -1;
else value = atof(line);

strtod would be better. You could then check for errors.

strncpy(fieldnames,name,MAXFIELDNAMELENGTH);


strncpy is rarely the right function for the job. It can leave you with
an array of chars which is NOT terminated by a '\0', so if you then
treat it as a string, which you do below, it will then go reading off in
to the wild blue yonder!
}

for(i = 0; i < numfields; i++){
error = 0;

do{
printf("type of \"%s\":",fieldnames);


void ui_tablemenu_query_new(void){
unsigned char numfields = table_numfields();
int i = 0;
void **v = NULL;
unsigned char type = 0;
char *fieldname = NULL;
int error = 0;
char cval = 0;
int ival = 0;
double dval = 0;

v = malloc(numfields*sizeof(void*));

You could use sizeof *v instead of sizeof(void*). It saves having to get
the type right or check further up that it is right.
if(v == NULL){
printf("ERROR: not enough memory\n");
return;
}

for(i = 0; i < numfields; i++){
type = table_fieldtype(i);
v = NULL;

switch(type){
case CHAR: v = malloc(sizeof(char)); break;
case INT: v = malloc(sizeof(int)); break;
case DOUBLE: v = malloc(sizeof(double)); break;
}


This is going to be very memory (and probably time) inefficient on most
systems. You might want to investigate using a union of char, int &
double to avoid this extra block of mallocs. If you also need pointers
to strings (which I think you do) then add a char* to the union as well.

<snip>


Well, that's as far as I've read. However, it does give you a few more
things to look.
 
S

s0suk3

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__

#define TABLE_OK 0
#define TABLE_ALREADY_EXISTS 1
#define TABLE_ERROR -1

void ui_mainmenu(void);
#endif

tree.h:

#ifndef __MYTREE_H__
#define __MYTREE_H__

#define MAXFIELDNAMELENGTH 100
#define SIGLENGTH 6
#define SIG "DBTBL"

#define FILECORRUPT -1

typedef struct __mytreeinfo__ {
        unsigned char numfields;
        unsigned long numrecords;
        unsigned char *afieldtypes;
        char **afieldnames;

}treeinfo_t;

typedef struct __mytreenode__ {
        struct __mytreenode__ *parent;
        struct __mytreenode__ *left;
        struct __mytreenode__ *right;
        void **field;

}treenode_t;

My suggestion is to put your struct definitions in source files, not
in header files, so as to make them opaque. Any member access and
operations should be provided by functions of the module.

Sebastian
 
F

Franken Sense

In Dread Ink, the Grave Hand of dfighter Did Inscribe:
#ifndef __MYTREE_H__
#define __MYTREE_H__

The initial doublescores are useless. The trailing doublescores are
useless.
 
K

Keith Thompson

Franken Sense said:
In Dread Ink, the Grave Hand of dfighter Did Inscribe:


The initial doublescores are useless. The trailing doublescores are
useless.

They're not exactly useless; __MYTREE_H__, __MYTREE_H, MYTREE_H__, and
MYTREE_H are four distinct identifiers. But identifiers with two
leading underscores are reserved to the implementation, and should
never be used in user code.
 
G

Guest

static void ui_trim(char *s){
int i = 0;
for(i = 0; s != '\0'; i++)
;


could this be replaced with
i = strlen(s);
while(i > 0 && !isalnum(s))
i--;

s[i+1] = '\0';
}


if you trim a string with no alphanums in it yeilds a string of length
1.
ui_trim("***") => "*"
is that ok?
[the string literal is notational convenience]

It will step off the end of the string if you pass it the empty string
 
F

Franken Sense

In Dread Ink, the Grave Hand of Keith Thompson Did Inscribe:
They're not exactly useless; __MYTREE_H__, __MYTREE_H, MYTREE_H__, and
MYTREE_H are four distinct identifiers. But identifiers with two
leading underscores are reserved to the implementation, and should
never be used in user code.

But none of this will be defined, unless he does so himself. His five
underscores are five useless keystrokes.
--
Frank

A concussion.
~~ Al Franken, Playboy interview, when asked what it would take for his
name to appear on the masthead of National Review or The Weekly Standard.
 
J

jameskuyper

Franken said:
In Dread Ink, the Grave Hand of Keith Thompson Did Inscribe:


But none of this will be defined, unless he does so himself. ...

That's not true of __MYTREE_H__ and __MYTREE_H. Since those
identifiers are reserved to the implementation, it's entirely possible
(though admitedly unlikely) that the implementation will already be
using those identifiers. That's what the reserved namespaces are for -
so implementations can use them. And that's precisely why the behavior
is undefined if you use them. The implementation might be using them
in a way incompatible with your use of them.
... His five
underscores are five useless keystrokes.

I don't follow your logic. I presume that you prefer MYTREEH? But by
the same exact argument, the 'M' is also useless, because YTREEH
"won't be defined, unless he does so himself".

The purpose of an identifier is to uniquely identify something. Every
significant character that makes up an identifiers serves that purpose
equally, because they all serve to distinguish that identifier from a
one which lacks that character. Note: only of the first 31 characters
of an external identifier and the first 63 characters of internal
identifiers need be significant, in this sense.
 
C

CBFalconer

Franken said:
In Dread Ink, the Grave Hand of Keith Thompson Did Inscribe:

But none of this will be defined, unless he does so himself.
His five underscores are five useless keystrokes.

You don't understand. If the file is #included twice, the first
inclusion will define that variable (without the horrible leading
underscores). The second inclusion will find the first definition,
and skip the file contents.
 
P

Phil Carmody

Franken Sense said:
In Dread Ink, the Grave Hand of Keith Thompson Did Inscribe:


But none of this will be defined, unless he does so himself. His five
underscores are five useless keystrokes.

Using that logic, the M, Y, T, R, Es, and H are equally useless.

Phil
 
P

Phil Carmody

Phil Carmody said:
Using that logic, the M, Y, T, R, Es, and H are equally useless.

Actually, using that logic, we can conclude 1 == 0, as your presumptions
are clearly false, and therefore anything can be derived from them.
__MYTREE_H__ and __MYTREE_H may well be defined, as names with such
forms are reserved for the implementation to do with what they wish.

Phil
 
B

Barry Schwarz

Dear everyone!
dfighterdb (I started it as mytree), is a tiny and simple console based
"database" application I'm writing as a learning project.
It's purpose is to teach me some generic dynamic data structures, and
general design of "larger" projects (Basically a binary tree and a
linear list with 256 generic fields).
It's not complete yet, however it is complete enough to present an
insight into my "coding style", to show how well (or badly) I can use
the language, how portable the code is, general design of the project, etc.
So I'd like to ask you all if you have the time to review it and comment
on it.

zipped source code + win32 binary:
http://www.freeweb.hu/dfighter/dfighterdb.zip

In tree.c, you have a instances of code of the form
ptr = NULL;
ptr = malloc(...);
-There is no point in assigning a variable one value and in the very
next statement assigning it a new value.

You have many identifiers that start with a double underscore. These
are reserved for the implementation. You shouldn't use them.

In common.c you have code of the form
case CHAR:{
if(*(char*)val1 > *(char*)val2)
retval = 1;
else
if(*(char*)val1 < *(char*)val2)
retval = -1;
}break;
-In most of your code I looked at, your closing braces line up with
the keyword to which the opening brace is attached. This makes it
easy to find the end of a block. Here the closing brace is well
indented beyond the case keyword and much too easy to miss, partly
because of the attached break.
-Furthermore, the break is not part of the if or the else or even the
initial if. It should not be on the same line as the brace and
aligned with the initial if since it is at the same "top" level within
the case.
-Your code uses tabs for indentation. If you ever have to post a
single function to Usenet asking for help, the tabs will play havoc
with your message. It is better(tm) to get used to using consistent
spacing for indenting your code.

Member field of treenode_t uses a dynamically allocated array of
void*. Every time you need to reference the object pointed to by one
of the void*, you cast the pointer to the appropriate type. In
addition to the intended effects, casts also tell the compiler it
should ignore certain problems which is almost always undesirable. I
wonder if a union of three pointer types would be easier on the typing
and reading. Something along the lines of
typedef union mypointers{
char* mychar;
int* myint;
double* mydouble;
}mypointer_t;
and in treenode_t
mypointer_t *field
Then in tree_dump the body of your switch would look like
case CHAR: printf("%c ",*root->field.mychar); break;
case INT: printf("%d ",*root->field.myint); break;
case DOUBLE: printf("%lf ",*root->field.mydouble); break;

In ui_mainmenu, you print out a line that does not terminate with a
'\n'. On some systems, this line may not appear in a timely fashion.
You should use
fflush(stdout);
to insure that it does.

If you are going to include several functions in a single translation
unit (.c file), consider putting them in alphabetical order to make it
easier to find. I recommend that functions larger than one screen
display should be in their own files.

In ui_mainmenu, you call ui_getmenuchoice to decide what to do. You
then test the returned value against EOF. ui_getmenuchoice cannot
possibly return the value EOF. Furthermore, ui_getmenuchoice does not
test the return from fgets for errors.

Why did you decide to use atof in ui_getINT when atoi seems more
appropriate? The cast is unnecessary. I recommend using the strto_
functions in place of atof simply for the better error detection.

Why does ui_getDOUBLE tolerate negative values while ui_getINT does
not?

I understand why ui_trim strips off trailing blanks but why does it
strip off trailing punctuation?

Why does ui_getSTRING refuse to recognize a string that starts with
something other than a letter?

In ui_tablemenu_create, once ui_getINT sets error to 1, it never gets
reset back to 0. You have the same problem when calling ui_getSTRING.

In ui_tablemenu_query_new, shy do you cast the return from
ui_getmenuchoice to unsigned char only to assign the result to cval
which is of type char? And the same problem with error.
 
G

Guest

Actually, using that logic, we can conclude 1 == 0, as your presumptions
are clearly false, and therefore anything can be derived from them.
__MYTREE_H__ and __MYTREE_H may well be defined, as names with such
forms are reserved for the implementation to do with what they wish.

yes we know that. But why "five useless keystrokes"? At least 3 of
the
underscores are problem free.
 
D

dfighter

CBFalconer said:
FYI Han is a troll. He has even announced that his objective is to
destroy c.l.c. Best plonked and ignored.
If you don't mind I'm more interested in productive discussion about C,
than the "politics" of c.l.c. I had my "tango" with Han, I told him what
I had to say about the topics we discussed. I'd like to leave it as that.

dfighter
 
D

dfighter

Keith said:
dfighter said:
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.)
Thank you Keith. I've seen that notation (-1) in M$' implementation of
the standard C library and I was actually wondering why they used that
instead of just -1. Now I know :)
Ofc I will change both the include guards and the define(s) as you
suggested.

dfighter
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top