Michael said:
I'm still learning C so I've written a simple app which lets you make a
contact list (stored as a linked list of structs), write it to a file, and
read it back. It works fine, but I notice in my loading procedure there's
a point or two where I'll malloc a struct which I then don't need, and
dispose of it; I was wondering if there was some way I could optimize this
kludge out of existence:
void loadContacts () {
int fd, bytesread;
if ( (fd = open("contacts.data", O_RDONLY, 0) ) != -1 ) {
struct contact *prev, *curr = (struct contact *)malloc(sizeof(struct contact));
if ( curr ) {
if ( ( bytesread = read(fd, curr, sizeof(struct contact))) == sizeof(struct contact) ) {
first.next = curr; // First is a global variable which is the first struct in the linked list
} else {
free (curr); // Can I avoid having to do this?
}
do {
prev = curr;
curr = (struct contact *)malloc(sizeof(struct contact));
prev->next = curr;
} while ( ( bytesread = read(fd, curr, sizeof(struct contact))) == sizeof(struct contact) );
if ( bytesread < sizeof(struct contact) ) {
prev->next = NULL;
free(curr); // Likewise, this?
}
} else {
perror("malloc.\n");
exit(1);
}
close (fd);
}
}
Creating functions for doubly linked lists, for reading and writing
them to a file, is a big chore for someone not experience in the
language. Looking at your posted function, I see that you must have
some understanding. However, the function design and logic flow
is prone for error. You need to evaluate your design and start
using ways to make a function do a specific task, and be useful
in communicating information with other functions. You can do this
by designing your function with parameters and a return value.
You need to redo the function and use the Standard C functions
fopen, fread, and fwrite in place of functions open, read and write.
Here is an example of how you might design various functions that
work together to reach your goal. Doing it this way makes managing
each function, and debugging, much easier.
This example creates a linked list, writes and reads as a binary file.
A good exercise would be to change the functions to read and write
the data in a text file. Then, if you are going to have a large
amount of data, you can look into ways of optimizing, such as
reading, writing, and allocating in chunks.
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
typedef struct CONTACT
{
char name[40];
char addr[40];
struct CONTACT *previous;
struct CONTACT *next;
} CONTACT;
CONTACT *CreateNODE(const char *name, const char *addr);
void PrintLIST(CONTACT *p);
void InsertLink(CONTACT **root, CONTACT *src);
void PrintNODE(CONTACT *p);
void FreeLIST(CONTACT **root);
int ListToFile(CONTACT *list, const char *filename);
CONTACT *FileToList(const char *filename);
int main(void)
{
CONTACT *Listhead = NULL, *tmp;
if((tmp = CreateNODE("George Washington","White House")) == NULL)
exit(EXIT_FAILURE);
InsertLink(&Listhead,tmp);
if((tmp = CreateNODE("Bill Clinton","Jail House")) == NULL)
exit(EXIT_FAILURE);
InsertLink(&Listhead,tmp);
if((tmp = CreateNODE("George Bush","Hot House")) == NULL)
exit(EXIT_FAILURE);
InsertLink(&Listhead,tmp);
PrintLIST(Listhead);
if(ListToFile(Listhead, "test.txt"))
puts("\nWrote the file to disk\n");
FreeLIST(&Listhead);
if((Listhead = FileToList("test.txt")) != NULL)
puts("\nRead the File into List\n");
PrintLIST(Listhead);
FreeLIST(&Listhead);
return 0;
}
CONTACT *CreateNODE(const char *name, const char *addr)
{
CONTACT *p;
if((p = malloc(sizeof *p)) == NULL) return NULL;
strncpy(p->name,name,sizeof p->name);
p->name[39] = '\0';
strncpy(p->addr,addr,sizeof p->addr);
p->addr[39] = '\0';
p->next = p->previous = NULL;
return p;
}
void PrintNODE(CONTACT *p)
{
printf("Name: %s\nAddress: %s\n\n",p->name,p->addr);
return;
}
void InsertLink(CONTACT **root, CONTACT *src)
{
CONTACT *tmp = *root;
if(tmp != NULL)
{
for( ; tmp->next; tmp = tmp->next);
src->previous = tmp;
tmp->next = src;
}
else *root = src;
return;
}
void PrintLIST(CONTACT *root)
{
size_t i = 1;
if(root)
{
for( ; root; root = root->next)
{
printf("Link: %u\n",i++);
PrintNODE(root);
}
}
return;
}
void FreeLIST(CONTACT **root)
{
CONTACT *tmp;
for( ; *root; *root = tmp)
{
tmp = (*root)->next;
free(*root);
}
}
int ListToFile(CONTACT *list, const char *filename)
{
FILE *fp;
if(list == NULL) return 0;
if((fp = fopen(filename,"wb")) == NULL) return 0;
for( ; list; list = list->next)
{
if(1 != fwrite(list,sizeof *list,1,fp))
{
remove(filename);
return 0;
}
}
fclose(fp);
return 1;
}
CONTACT *FileToList(const char *filename)
{
CONTACT *root = NULL, tmp , *ptmp;
FILE *fp;
size_t i;
if((fp = fopen(filename, "rb")) == NULL) return NULL;
while((i = fread(&tmp,sizeof tmp, 1, fp)) == 1)
{
if((ptmp = CreateNODE(tmp.name,tmp.addr)) == NULL)
{
FreeLIST(&root);
return NULL;
}
InsertLink(&root,ptmp);
}
if(feof(fp) == 0)
{
FreeLIST(&root);
return NULL;
}
fclose(fp);
return root;
}