Structure Clobering

jInuQ · 05-17-2004, 02:26 AM

I am writing a file integrity program. I first scans through all of the directories, gathering file names. The problem I am having is in storing the file name and directory names. File names seem to bleed into the directory tree. The first 0a-9a directories are correct ,but once I hit the bottom of the fuction the files have bleed through. Attached is the entire source code, if you feel the the code sections I have posted are not enough.

Code:

 D:\tmp\0a\
 D:\tmp\1a\
 D:\tmp\2a\
 D:\tmp\3a\
 D:\tmp\4a\
 D:\tmp\5a\
 D:\tmp\6a\
 D:\tmp\7a\
 D:\tmp\8a\
 D:\tmp\9a\
 D:\tmp\u.txt
 D:\tmp\v.txt
 D:\tmp\w.txt
 D:\tmp\x.txt
 D:\tmp\y.txt
 D:\tmp\z.txt
 D:\tmp\6a\
 D:\tmp\7a\
 D:\tmp\8a\
 D:\tmp\9a\

Code:

 struct File {
 	
 	int i_files;
 	int a_files;
 	char **files;
 };
 
 struct Directory {
 	
 	int i_dirs;
 	int a_dirs;
 	char **dirs;
 };

The following code add directories to a Directory structure and files to a File structure. It does not show the initialization of the structures.

Code:

 	while ((file = readdir(dir_in)) != NULL) {
 
 		if (!((dir_in->dd_dta).attrib == _A_SUBDIR)) {
 
 			if (wfiles->i_files >= wfiles->a_files) {
 				wfiles->files =
 					realloc(wfiles->files,
 		    		    wfiles->a_files + MIN_FILE_ALLOC);
 				if (wfiles->files == NULL) {
 					fprintf(stderr,
 		    			"Something really bad just happend. We lost the file list in realloc.\n");
 					return NULL;
 				}
 				wfiles->a_files += MIN_FILE_ALLOC;
 			}
 
 			wfiles->files[wfiles->i_files] =
 				calloc(MAX_FILE_NAME, sizeof(char));
 			if (wfiles->files[wfiles->i_files] == NULL) {
 				fprintf(stderr, "File name too long.\n");
 				return NULL;
 			}
 			strncpy(wfiles->files[wfiles->i_files], directory,
 				DIR_IN_LEN);
 			strncpy(wfiles->files[wfiles->i_files] + DIR_IN_LEN,
 				"\\", 1);
 			strncpy(wfiles->files[wfiles->i_files] + DIR_IN_LEN + 1,
 				file->d_name, file->d_namlen);
 			wfiles->i_files++;
 
 		} else {
 
 			if (wdirs->i_dirs >= wdirs->a_dirs) {
 				wdirs->dirs =
 					realloc(wdirs->dirs,
 		    		    wdirs->a_dirs + MIN_FILE_ALLOC);
 				if (wdirs->dirs == NULL) {
 					fprintf(stderr,
 		    			"Something really bad just happend. We lost the directory list in realloc.\n");
 					return NULL;
 				}
 				wdirs->a_dirs += MIN_FILE_ALLOC;
 				printf("%i\t%i\n", wdirs->a_dirs,
 		    		   wdirs->i_dirs);
 			}
 
 			if (strcmp(file->d_name, ".") != 0
 				&& strcmp(file->d_name, "..") != 0) {
 				wdirs->dirs[wdirs->i_dirs] =
 		    		malloc(MAX_FILE_NAME * sizeof(char));
 				if (wdirs->dirs[wdirs->i_dirs] == NULL) {
 					fprintf(stderr,
 		    			"Unable to malloc(MAX_FILE_NAME * sizeof( char )).\n");
 					return NULL;
 				}
 				if (memset
 		    	    (wdirs->dirs[wdirs->i_dirs], '\0',
 					 MAX_FILE_NAME) == NULL) {
 					fprintf(stderr,
 		    			"We had some problem with memset.\n");
 					return NULL;
 				}
 				strncpy(wdirs->dirs[wdirs->i_dirs], directory,
 					DIR_IN_LEN);
 		    	strncpy(wdirs->dirs[wdirs->i_dirs] + DIR_IN_LEN,
 					"\\", 1);
 				strncpy(wdirs->dirs[wdirs->i_dirs] +
 					DIR_IN_LEN + 1, file->d_name,
 					file->d_namlen);
 				strncpy(wdirs->dirs[wdirs->i_dirs] +
 		    		DIR_IN_LEN + file->d_namlen + 1, "\\",
 					1);
 				printf("%s\n", wdirs->dirs[wdirs->i_dirs]);
 				wdirs->i_dirs++;
 			}
 		}
 
 	}

anonytmouse · 05-17-2004, 03:41 AM

You are experiencing the unpredictable results of a buffer overrun.

Code:

wfiles->files = realloc(wfiles->files, wfiles->a_files + MIN_FILE_ALLOC);

should be:

Code:

 wfiles->files = realloc(wfiles->files, (wfiles->a_files + MIN_FILE_ALLOC) * sizeof(*wfiles->files));

The same for other places where you allocate memory.

Welcome to the forums!

**Dave_Sinkula** · 05-17-2004, 08:21 AM

Also, using realloc in the following manner is not a good thing to do.

Code:

ptr = realloc(ptr, elements * sizeof *ptr);

It is better to use another temporary pointer.

Code:

temp = realloc(ptr, elements * sizeof *ptr);
if ( temp != NULL )
{
   ptr = temp;
}
else
{
   free(ptr);
   /* etc. */
}

Then if the realloc call failed, you don't leak the memory you started with.

jInuQ · 05-17-2004, 05:20 PM

Dave_Sinkula:
Thanks for the reminder. It slipped my mind completly. 8^)

anonytmouse:
I added you suggestion and it did not change anything. I do need to add a sizeof but for char, as the wfiles->a_files variable holds how many chars have been allocated. Thanks for the help.

anonytmouse · 05-18-2004, 12:14 AM

>> I do need to add a sizeof but for char, as the wfiles->a_files variable holds how many chars have been allocated. <<

No, wfiles->files holds an array of character pointers. Therefore, you have to multiply the desired number of entries by sizeof(char *). Multiplying by sizeof(char) will give you one quarter of the memory that you need(char being one byte while char * is 4 bytes on Win32). This same error is made in your calls to calloc.

These are all wrong:

Code:

	wfiles->files = calloc(MIN_FILE_ALLOC, sizeof(char));
	wdirs->dirs = calloc(MIN_FILE_ALLOC, sizeof(char));

	char **test = calloc(MIN_FILE_ALLOC, sizeof(char));
	for (int a = 0; a < MIN_FILE_ALLOC; a++) {
		test[a] = calloc(MAX_FILE_NAME, sizeof(char));
	}

Check all your calls to calloc(), malloc() and realloc().

jInuQ · 05-18-2004, 02:35 AM

I feel soooo very slow some times. 8^) Thanks for pointing that out. I will go through the code and see if that fixes it.

[edit] Now I feel really slow as that seems to have fixed it. Thanks for all the help.[/edit]

jInuQ

05-17-2004, 03:41 AM	#2
anonytmouse Yes, my avatar is stolen Join Date: Dec 2002 Posts: 2,544	You are experiencing the unpredictable results of a buffer overrun. Code: wfiles->files = realloc(wfiles->files, wfiles->a_files + MIN_FILE_ALLOC); should be: Code: wfiles->files = realloc(wfiles->files, (wfiles->a_files + MIN_FILE_ALLOC) * sizeof(*wfiles->files)); The same for other places where you allocate memory. Welcome to the forums!
anonytmouse is offline

05-17-2004, 08:21 AM	#3
Dave_Sinkula Just Lurking Join Date: Oct 2002 Posts: 5,006	Also, using realloc in the following manner is not a good thing to do. Code: ptr = realloc(ptr, elements * sizeof ptr); It is better to use another temporary pointer. Code: temp = realloc(ptr, elements sizeof ptr); if ( temp != NULL ) { ptr = temp; } else { free(ptr); / etc. / } Then if the realloc call failed, you don't leak the memory you started with. __________________ 7. It is easier to write an incorrect program than understand a correct one. 40. There are two ways to write error-free programs; only the third one works.
Dave_Sinkula is offline

05-17-2004, 05:20 PM	#4
jInuQ Registered User Join Date: May 2004 Posts: 3	Dave_Sinkula: Thanks for the reminder. It slipped my mind completly. 8^) anonytmouse: I added you suggestion and it did not change anything. I do need to add a sizeof but for char, as the wfiles->a_files variable holds how many chars have been allocated. Thanks for the help.
jInuQ is offline

05-18-2004, 12:14 AM	#5
anonytmouse Yes, my avatar is stolen Join Date: Dec 2002 Posts: 2,544	>> I do need to add a sizeof but for char, as the wfiles->a_files variable holds how many chars have been allocated. << No, wfiles->files holds an array of character pointers. Therefore, you have to multiply the desired number of entries by sizeof(char ). Multiplying by sizeof(char) will give you one quarter of the memory that you need(char being one byte while char is 4 bytes on Win32). This same error is made in your calls to calloc. These are all wrong: Code: wfiles->files = calloc(MIN_FILE_ALLOC, sizeof(char)); wdirs->dirs = calloc(MIN_FILE_ALLOC, sizeof(char)); char **test = calloc(MIN_FILE_ALLOC, sizeof(char)); for (int a = 0; a < MIN_FILE_ALLOC; a++) { test[a] = calloc(MAX_FILE_NAME, sizeof(char)); } Check all your calls to calloc(), malloc() and realloc().
anonytmouse is offline

05-18-2004, 02:35 AM	#6
jInuQ Registered User Join Date: May 2004 Posts: 3	I feel soooo very slow some times. 8^) Thanks for pointing that out. I will go through the code and see if that fixes it. [edit] Now I feel really slow as that seems to have fixed it. Thanks for all the help.[/edit] jInuQ Last edited by jInuQ; 05-18-2004 at 02:40 AM.
jInuQ is offline

Thread Tools
Show Printable Version Email this Page
Display Modes
Linear Mode Switch to Hybrid Mode Switch to Threaded Mode

Similar Threads
Thread	Thread Starter	Forum	Replies	Last Post
Problem referencing structure elements by pointer	trillianjedi	C Programming	19	06-13-2008 05:46 PM
How to call a function several times, but with a different structure as argument?	mabuhay	C Programming	5	02-14-2006 09:04 AM
Dikumud	maxorator	C++ Programming	1	10-01-2005 06:39 AM
Serial Communications in C	ExDigit	Windows Programming	7	01-09-2002 10:52 AM
C structure within structure problem, need help	Unregistered	C Programming	5	11-30-2001 05:48 PM