Breaking up source code into multiple files

Ok, I'm messing around trying to learn C and got my code working but it was just one large .c file. Now I wanted to break it up into multiple files so it would be easier to see what's going on but ran into some issues. I would appreciate any help with this.

main.c

Code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "functions.h"

int main()
{
	char *name;
	name = strdup_func("Brian Gable");
	struct employee *eptr = malloc(sizeof *eptr);
	
	build_employee( eptr);
	printf("Empolyee name is: %s\n",eptr->name);
	printf("Empolyee age is: %d\n",eptr->age);
	printf("Empolyee address is: %s\n",eptr->address);
	strcpy(eptr->name,name);
	printf("Empolyee name is: %s\n",eptr->name);
	printf("Empolyee age is: %d\n",eptr->age);
	printf("Empolyee address is: %s\n",eptr->address);
	name = substrg(name,1,5);
	printf("Substring: %s\n", name);
	name = reverse_string(name);
	printf("Substring reversed: %s\n", name);
	
	return (0);	
}

functions.c

Code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "type.h"

void build_employee(struct employee *employee_pointer)
{
	strcpy(employee_pointer->name, "Michael Clark");
	employee_pointer->age = 29;
	strcpy(employee_pointer->address, "3302 Runner Rd Apt 107");
	
}

char *strdup_func(char *instr)
{
	char *outstr;
	if (!instr)
	{
		fprintf(stderr, "strdup: FATAL - NULL argument!\n");
		return(NULL);
	}
	
	outstr = (char *) malloc(sizeof(char) * (strlen(instr)+1) );
	if (!outstr)
	{
		fprintf(stderr, "strdup: FATAL - malloc failed!");
		return(NULL);
	}
	
	strcpy(outstr, instr);
	
	return(outstr);
}

char *substrg(char *first_string, int start, int numchars)
{
	char *p;
	
	int count = 0, length = 0;
	
	length = strlen(first_string);
	
	if(start > length || length < 2 || start < 1)
	{
		return (NULL);
	}
	
	p = (char *) malloc(sizeof(char) * (numchars+1));
	
	start--;
	
	while (count < numchars)
	{
		p[count]= *(first_string+start+count);
		count++;
	}
	p[count] = '\0';	
	return (p);
	
}

char *reverse_string(char *string)
{
	char *p;
	int slen = 0, count = 0;
	slen = strlen(string);
	p = (char *) malloc(sizeof(char) * (slen+1));
	
	while (string[count] != '\0')
	{
		p[--slen]=string[count++];
	}
	
	return (p);
}
void update_employee_name(struct employee *employee_pointer, char new_name[80])
{
	strcpy(employee_pointer->name, new_name);
}

functions.h

Code:

#ifndef FUNC_H
#define FUNC_H

void build_employee(struct employee *employee_pointer);
char *strdup_func(char *instr);
char *substrg(char *first_string, int start, int numchars);
char *reverse_string(char *string);
void update_employee_name(struct employee *employee_pointer, char new_name[80]);

#endif

type.h

Code:

#ifndef TYPE_H
#define TYPE_H

struct employee 
{
    char name[80];
    int age;
    char  address[256];
};

extern struct employee *eptr;

#endif

errors

Code:

main.C: In function ‘int main()’:
main.C:9: warning: deprecated conversion from string constant to ‘char*’
main.C:10: error: invalid application of ‘sizeof’ to incomplete type ‘employee’ 
main.C:10: error: invalid conversion from ‘void*’ to ‘employee*’
main.C:13: error: invalid use of incomplete type ‘struct employee’
functions.h:4: error: forward declaration of ‘struct employee’
main.C:14: error: invalid use of incomplete type ‘struct employee’
functions.h:4: error: forward declaration of ‘struct employee’
main.C:15: error: invalid use of incomplete type ‘struct employee’
functions.h:4: error: forward declaration of ‘struct employee’
main.C:16: error: invalid use of incomplete type ‘struct employee’
functions.h:4: error: forward declaration of ‘struct employee’
main.C:17: error: invalid use of incomplete type ‘struct employee’
functions.h:4: error: forward declaration of ‘struct employee’
main.C:18: error: invalid use of incomplete type ‘struct employee’
functions.h:4: error: forward declaration of ‘struct employee’
main.C:19: error: invalid use of incomplete type ‘struct employee’
functions.h:4: error: forward declaration of ‘struct employee’

Yikes. My best guess would be that you're not compiling it right.

Originally Posted by Babkockdood

Yikes. My best guess would be that you're not compiling it right.

Maybe, but I really think I have a mistake....I'm compiling on my mac in the terminal using gcc.

Code:

gcc main.c functions.c -o myprog

I think it's much easier to arrange your source by module.

By that, I mean separate your file so that one h file declares employee and declares the function prototypes of the things employee uses, one C file implements (defines) things employee uses (and perhaps defines employee itself), and one C file for main, et cetera. You can keep going but generally you want separate files for types and then you have a tools file or something.

So, one mistake is that you have an h file with ALL the prototypes in it and you didn't #include "type.h". You also didn't include "functions.h" in functions.c -- which means functions.c has no prototypes unless things are compiled in a specific order (GLHF).

Contrast this with the earlier modular order: employee files include everything employee needs to work/be implemented, and program files include everything they use.

Example employee module:

Code:

#ifndef EMPLOYEE_H
#define EMPLOYEE_H
/* employee.h */
extern struct employee;
void build_employee(struct employee *employee_pointer);
void update_employee_name(struct employee *employee_pointer, char new_name[80]);
#endif

/* employee.c */
#include <string.h>
#include "employee.h"
struct employee 
{
    char name[80];
    int age;
    char  address[256];
};

void build_employee(struct employee *employee_pointer)
{
	strcpy(employee_pointer->name, "Michael Clark");
	employee_pointer->age = 29;
	strcpy(employee_pointer->address, "3302 Runner Rd Apt 107");
	
}

void update_employee_name(struct employee *employee_pointer, char new_name[80])
{
	strcpy(employee_pointer->name, new_name);
}

HTH.

I guess I did compile it wrong. I typed the ".c" as ".C". I have different errors now.

Code:

In file included from example.c:4:
functions.h:4: warning: ‘struct employee’ declared inside parameter list
functions.h:4: warning: its scope is only this definition or declaration, which is probably not what you want
functions.h:8: warning: ‘struct employee’ declared inside parameter list
main.c: In function ‘main’:
main.c:10: error: dereferencing pointer to incomplete type
main.c:12: warning: passing argument 1 of ‘build_employee’ from incompatible pointer type
main.c:13: error: dereferencing pointer to incomplete type
main.c:14: error: dereferencing pointer to incomplete type
main.c:15: error: dereferencing pointer to incomplete type
main.c:16: error: dereferencing pointer to incomplete type
main.c:16: error: dereferencing pointer to incomplete type
main.c:16: error: dereferencing pointer to incomplete type
main.c:16: error: dereferencing pointer to incomplete type
main.c:17: error: dereferencing pointer to incomplete type
main.c:18: error: dereferencing pointer to incomplete type
main.c:19: error: dereferencing pointer to incomplete type

Originally Posted by whiteflags

I think it's much easier to arrange your source by module.

By that, I mean separate your file so that one h file declares employee and declares the function prototypes of the things employee uses, one C file implements (defines) things employee uses (and perhaps defines employee itself), and one C file for main, et cetera. You can keep going but generally you want separate files for types and then you have a tools file or something.

So, one mistake is that you have an h file with ALL the prototypes in it and you didn't #include "type.h". You also didn't include "functions.h" in functions.c -- which means functions.c has no prototypes unless things are compiled in a specific order (GLHF).

Contrast this with the earlier modular order: employee files include everything employee needs to work/be implemented, and program files include everything they use.

Example employee module:

Code:

#ifndef EMPLOYEE_H
#define EMPLOYEE_H
/* employee.h */
extern struct employee;
void build_employee(struct employee *employee_pointer);
void update_employee_name(struct employee *employee_pointer, char new_name[80]);
#endif

/* employee.c */
#include <string.h>
#include "employee.h"
struct employee 
{
    char name[80];
    int age;
    char  address[256];
};

void build_employee(struct employee *employee_pointer)
{
	strcpy(employee_pointer->name, "Michael Clark");
	employee_pointer->age = 29;
	strcpy(employee_pointer->address, "3302 Runner Rd Apt 107");
	
}

void update_employee_name(struct employee *employee_pointer, char new_name[80])
{
	strcpy(employee_pointer->name, new_name);
}

HTH.

Thanks!

I changed it to what you suggested, however I am still getting those errors in main.c for dereferencing pointer to incomplete type. Why am I getting that error?

>>Why am I getting that error?
I missed this when I posted what you replied to but you also didn't #include "type.h" to use employee in main.c. You want the main file to know what struct employee * is so you include it, otherwise compilation depends on the order you compile files. You definitely still need to do that if you followed my advice.

Originally Posted by me

Contrast this with the earlier modular order: employee files include everything employee needs to work/be implemented, and program files include everything they use.

Where program files would be main.c and everything that is not employee.c or employee.h (or other "type" files). I hope that's clearer now.

Thanks a lot! I do have another question.

I keep getting this warning for three lines, assignment makes pointer from integer without a cast. Here is one of the lines it is referring to:

Code:

name = strdup_func("Brian Gable");

Now when I run the program I get a segmentation fault. I try to fix it and the warnings go away but then I get a bus error. So I'm not sure if I'm fixing it right. Can you help me out with this little issue? I appreciate the help.

#1.

Code:

char *strdup_func(char *instr)
{
    char *outstr;
    if (!instr)
    {
        fprintf(stderr, "strdup: FATAL - NULL argument!\n");
        return(NULL);
    }
    
    outstr = (char *) malloc(sizeof(char) * (strlen(instr)+1) );

    ...

char *substrg(char *first_string, int start, int numchars)
{
    char *p;
	
    int count = 0, length = 0;
	
    length = strlen(first_string);
	
    if(start > length || length < 2 || start < 1)
    {
        return (NULL);
    }
	
    p = (char *) malloc(sizeof(char) * (numchars+1));

    ...

char *reverse_string(char *string)
{
    char *p;
    int slen = 0, count = 0;
    slen = strlen(string);
    p = (char *) malloc(sizeof(char) * (slen+1));
    ...

Casting the return value from the malloc call should be avoided. The function returns a void * and that should be converted safely to char *. Also, sizeof(char) is guaranteed to be 1 so multiplying anything by 1 does not change its value and that bit of code is therefore redundant. Those three lines should simply be:

Code:

outstr = malloc(strlen(instr)+1);

...

p = malloc(numchars+1);

...

p = malloc(slen+1);

#2 - Again with those same 3 functions:

Code:

char *strdup_func(char *instr);
char *substrg(char *first_string, int start, int numchars);
char *reverse_string(char *string);

None of those functions change the char* argument passed into them. For the sake of const correctness, they should probably be declared const:

Code:

char *strdup_func(const char *instr);
char *substrg(const char *first_string, int start, int numchars);
char *reverse_string(const char *string);

#3 You've called malloc in those 3 functions but there is nowhere in your program where you then call free to release that memory. This should be done just prior to exiting the program.



#4 Though you check for bad allocation in the strdup_func function, you fail to do so in the other 2 functions that call malloc.


#5

Originally Posted by mherald81

I keep getting this warning for three lines, assignment makes pointer from integer without a cast.

That usually occurs because the compiler doesn't have a definition/declaration of the function available at the point where the warning is issued. That is, the compiler doesn't know what the strdup_func looks like (most likely because the header hasn't been included prior to that point that does declare/define it). When this happens, it assumes a return type of int for strdup_func and then sees you are assigning this int to a char* and issues the warning you mention. This issue is usually fixed by properly including the necessary header. Post all your new code so we can see what's up.

Thanks guys! It works now. I will be messing around with some stuff over the weekend to learn some more...maybe implement a linked list or something. If I run into any problems I will post a thread here. Again, thanks for all the help!