ProgrammingThis forum is for all programming questions.
The question does not have to be directly related to Linux and any language is fair game.
Notices
Welcome to LinuxQuestions.org, a friendly and active Linux Community.
You are currently viewing LQ as a guest. By joining our community you will have the ability to post topics, receive our newsletter, use the advanced search, subscribe to threads and access many other special features. Registration is quick, simple and absolutely free. Join our community today!
Note that registered members see fewer ads, and ContentLink is completely disabled once you log in.
I have a programming problem that I hope someone out there can help me with. I am trying to learn C programming for a task at work and I have set myself a little project, which consists of reading down a file tree including all the sub directories obtaining information about each file. Such as the file name, size in bytes, the date last modified both since epoch and also in human readable form, then put the data into a MySQL database table.
The problem I get is half way through the execution I get a segmentation fault, and this occurs after putting in 5926 rows into the mysql table. Below is the code of the function that I have written to complete the reading of the file and putting into the database:
Code:
void genfilelist(char* path, char* role)
{
struct dirent* entry;
struct stat stbuf;
DIR* dir;
size_t path_len;
char* datain;
char* entry_path;
char dirpath[PATH_MAX];
//null dirpath string
int i;
for(i =0;i<PATH_MAX;i++){
dirpath[i] = 0;
}
strncpy(dirpath, path,strlen(path));
path_len = strlen(dirpath);
/*checks to see if path ends in a /. if not
*append /*/
if(dirpath[path_len -1] != '/'){
dirpath[path_len] = '/';
dirpath[path_len +1] = '\0';
++path_len;
}
//printf("Debug: %s Length: %d\n",dirpath,strlen(dirpath));
/*open directory*/
char* tmp = get_file_type(path);
dir = opendir(path);
while((entry = readdir(dir))!=NULL) {
const char* type;
/*build path to file by appending the name of the file to the path*/
//strcat(dirpath ,entry->d_name);
strncpy(dirpath + path_len, entry->d_name, sizeof(dirpath)-path_len);
/*if the directory path ends with either /. or /.. then ignore else compare
*the different file/directory*/
if(((dirpath[strlen(dirpath)-1] !='.' && (dirpath[strlen(dirpath)-2] !='/')) ||
(dirpath[strlen(dirpath)-1]!='.')&&(dirpath[strlen(dirpath)-2] != '.'))){
/*Determine file type*/
type = get_file_type(dirpath);
/*Enter directory if file type = directory*/
if(strcmp(type,"directory")==0){
char* next_path;
next_path = (char*)malloc(BUFFER);
if(next_path == NULL){
fprintf(stderr,"Can not allocate memory for string.\n");
}
int i = 0;
/*clear string array of crap*/
for(i;i<BUFFER;i++){
next_path[i] = 0;
}
strncat(next_path,dirpath,strlen(dirpath));
/*Recall function to drill down dir*/
genfilelist(next_path,role);
free(next_path);
close(dir);
}
datain = (char*)malloc(BUFFER);
if(datain ==NULL){
fprintf(stderr,"Can not allocate memory for string.\n");
}
/*Check to see if it is a file and get the file information*/
if(strncmp(type,"regular_file",strlen("regular_file"))==0){
off_t* fsize; //file size in bytes
time_t* lastMtime; //time last modified (epoch)
if(stat(dirpath,&stbuf) == -1){
fprintf(stderr,"can not access data on %s\n",dirpath);
}
/*Get file informaton of file.*/
fsize = stbuf.st_size;
lastMtime = stbuf.st_mtime;
/*Convert epoch time to human readable*/
struct tm* timehm;
timehm = localtime(&lastMtime);
/*build string(s) to store in the database*/
char* alterStr = (char*)malloc(BUFFER);
if(alterStr == NULL){
fprintf(stderr,"Can not allocate memory for string.\n");
}
sprintf(alterStr,"INSERT INTO %s (filename,datestamp,epochdate,size)values(\"%s\",\"%s\",\"%d\",\"%d\")\n",
role,dirpath,asctime(timehm),lastMtime,fsize);
printf("Debug: %s\n",alterStr);
int sucess = alter_database_data(alterStr);
if(sucess == 0){
fprintf(stderr,"MySQL Error:failed to insert data into %s table!\n",role);
}
/*free memory*/
free(datain);
free(alterStr);
}
}
}
if(dir != NULL){
close(dir);
}
}
At present the application is working through the /usr directory and is crashing after inserting into the database the following file: /usr/src/kernels/2.6.13-1.1526_FC4-smp-i686/include/config/pcmcia/qlogic/module.h
If anyone can point me in the right direction and also willing to give me pointers in improving my C programming I would be most grateful to you.
Many thanks in advance...
BTW I am running this on Fedora Core 4, and using kdevelop as my IDE.
I made a few changes in your code which may solve the problems that you are running in to. The changes are as follows:
1. close(dir) -> closedir(dir) There are two calls to close both of which should be closedir
2. closedir(dir) - originally close(dir) - removed. The first call to closedir() is not necessary and will make the program fail.
3. fsize & lastMtime should not be pointers.
4. free(datain) too early. This causes a memory leak
I'm going to follow up with some info on how I diagnosed this but these changes should get you going or at least help.
- Dave
Code:
void genfilelist(char* path, char* role)
{
struct dirent* entry;
struct stat stbuf;
DIR* dir;
size_t path_len;
char* datain;
char* entry_path;
char dirpath[PATH_MAX];
//null dirpath string
int i;
for(i =0;i<PATH_MAX;i++){
dirpath[i] = 0;
}
strncpy(dirpath, path,strlen(path));
path_len = strlen(dirpath);
/*checks to see if path ends in a /. if not
*append /*/
if(dirpath[path_len -1] != '/'){
dirpath[path_len] = '/';
dirpath[path_len +1] = '\0';
++path_len;
}
//printf("Debug: %s Length: %d\n",dirpath,strlen(dirpath));
/*open directory*/
char* tmp = get_file_type(path);
dir = opendir(path);
while((entry = readdir(dir))!=NULL) {
const char* type;
/*build path to file by appending the name of the file to the path*/
//strcat(dirpath ,entry->d_name);
strncpy(dirpath + path_len, entry->d_name, sizeof(dirpath)-path_len);
/*if the directory path ends with either /. or /.. then ignore else compare
*the different file/directory*/
if(((dirpath[strlen(dirpath)-1] !='.' && (dirpath[strlen(dirpath)-2] !='/')) ||
(dirpath[strlen(dirpath)-1]!='.')&&(dirpath[strlen(dirpath)-2] != '.'))){
/*Determine file type*/
type = get_file_type(dirpath);
/*Enter directory if file type = directory*/
if(strcmp(type,"directory")==0){
char* next_path;
next_path = (char*)malloc(BUFFER);
if(next_path == NULL){
fprintf(stderr,"Can not allocate memory for string.\n");
}
int i = 0;
/*clear string array of crap*/
for(i;i<BUFFER;i++){
next_path[i] = 0;
}
strncat(next_path,dirpath,strlen(dirpath));
/*Recall function to drill down dir*/
genfilelist(next_path,role);
free(next_path);
// closedir(dir); // FIXED ( closing directory here results in the dir being closed too soon)
}
datain = (char*)malloc(BUFFER);
if(datain ==NULL){
fprintf(stderr,"Can not allocate memory for string.\n");
}
/*Check to see if it is a file and get the file information*/
if(strncmp(type,"regular_file",strlen("regular_file"))==0){
off_t fsize; //file size in bytes // FIXED ( originally a pointer )
time_t lastMtime; //time last modified (epoch) // FIXED ( originally a pointer )
if(stat(dirpath,&stbuf) == -1){
fprintf(stderr,"can not access data on %s\n",dirpath);
}
/*Get file informaton of file.*/
fsize = stbuf.st_size;
lastMtime = stbuf.st_mtime;
/*Convert epoch time to human readable*/
struct tm* timehm;
timehm = localtime(&lastMtime);
/*build string(s) to store in the database*/
char* alterStr = (char*)malloc(BUFFER);
if(alterStr == NULL){
fprintf(stderr,"Can not allocate memory for string.\n");
}
sprintf(alterStr,"INSERT INTO %s (filename,datestamp,epochdate,size)values(\"%s\",\"%s\",\"%d\",\"%d\")\n",
role,dirpath,asctime(timehm),lastMtime,fsize);
printf("Debug: %s\n",alterStr);
int sucess = alter_database_data(alterStr);
if(sucess == 0){
fprintf(stderr,"MySQL Error:failed to insert data into %s table!\n",role);
}
/*free memory*/
free(alterStr);
}
free(datain); // FIXED ( moved to here from inside conditional )
}
}
if(dir != NULL){
closedir(dir); // FIXED ( was close(dir) )
}
}
The first thing I did with this code was put it in a file, add the appropriate include files and compile it in C. This gave warnings that showed the fsize and lastMtime pointer problems. These probably were not serious but it's good to fix these just in case.
Next, I switched to using the c++ compiler which has better error checking. This required more include files to be added. I commented out the call to alter_database_data() and included my own version of get_file_type() and a main() routine to run the routine on my filesystem. This resulted in the following compiler error:
Quote:
make orig
g++ -g orig.cpp -o orig
orig.cpp: In function `void genfilelist(char*, char*)':
orig.cpp:90: error: invalid conversion from `DIR*' to `int'
make: *** [orig] Error 1
This compile error was the hint that led me to realize that closedir should have been called instead of close.
At this point the program compiled cleanly.
When I ran the program, it went into an infinite loop for reasons that were not clear. I ran it under gdb which didn't show me anything interesting right off and then tried another debugging tool, valgrind:
Quote:
valgrind --log-file=orig.vg ./orig
The following valgrind error showed up in the log file:
Quote:
==19212== Invalid read of size 4
==19212== at 0x1BA98461: readdir (in /lib/tls/i686/cmov/libc-2.3.2.so)
==19212== by 0x80488D9: genfilelist(char*, char*) (orig.cpp:59)
==19212== by 0x8048A49: genfilelist(char*, char*) (orig.cpp:88)
==19212== by 0x8048C05: main (orig.cpp:148)
==19212== Address 0x1BB294C0 is 24 bytes inside a block of size 4124 free'd
==19212== at 0x1B90068F: free (vg_replace_malloc.c:235)
==19212== by 0x1BA983F7: closedir (in /lib/tls/i686/cmov/libc-2.3.2.so)
==19212== by 0x8048A62: genfilelist(char*, char*) (orig.cpp:90)
==19212== by 0x8048A49: genfilelist(char*, char*) (orig.cpp:88)
==19212== by 0x8048C05: main (orig.cpp:148)
Seeing this focused my attention on the opendir/readdir/closedir combinations and I realized that there was an extra closedir. When I took this out the program worked. Valgrind also showed the memory leak caused by not always freeing datain.
I don't know if you will find this useful or not but once you get the code to compile in c++ you can take advantage of the standard c++ string type to simplify almost all of the string allocation and manipulation that is in your genfilelist routine.
As I mentioned in my first posting, using the c++ compiler on c code generally requires no changes to the code other than cleanup that is generally a good thing to do anyway. This was what clued me in to the incorrect use of close() in your program.
Here is an updated version of genfilelist() that uses string instead of char* and char[] . Note that there are no more malloc and free calls and all the calls to strcat, strcmp, strlen, strcpy and the tricky checking for "/." and "/.." have been replaced by more readable string operations. Despite these change, the code is basically the same.
- Dave
Code:
void genfilelist(const string& dirpath, char* role)
{
struct dirent* entry;
struct stat stbuf;
DIR* dir;
dir = opendir(dirpath.c_str());
while((entry = readdir(dir))!=NULL) {
string entryName = entry->d_name;
/*if the directory path ends with either /. or /.. then ignore else compare
*the different file/directory*/
if(entryName != "." && entryName != "..") {
/*build path to file by appending the name of the file to the path*/
string entryPath = dirpath + "/" + entryName;
/*Determine file type*/
string type = get_file_type(entryPath.c_str());
/*Enter directory if file type = directory*/
if(type == "directory"){
genfilelist(entryPath, role);
}
/*Check to see if it is a file and get the file information*/
if(type == "regular_file"){
off_t fsize; //file size in bytes
time_t lastMtime; //time last modified (epoch)
if(stat(entryPath.c_str(),&stbuf) == -1){
fprintf(stderr,"can not access data on %s\n",entryPath.c_str());
}
/*Get file informaton of file.*/
fsize = stbuf.st_size;
lastMtime = stbuf.st_mtime;
/*Convert epoch time to human readable*/
struct tm* timehm;
timehm = localtime(&lastMtime);
/*build string(s) to store in the database*/
stringstream alterStr;
alterStr << "INSERT INTO "
<< role
<< " (filename,datestamp,epochdate,size)values(\""
<< dirpath << "\",\""
<< asctime(timehm) << "\",\""
<< lastMtime << "\",\""
<< fsize << "\")" << endl;
cout << "Debug: " << alterStr.str() << endl;
int sucess = alter_database_data(alterStr.str().c_str());
if(sucess == 0){
fprintf(stderr,"MySQL Error:failed to insert data into %s table!\n",role);
}
}
}
}
if(dir != NULL){
closedir(dir);
}
}
LinuxQuestions.org is looking for people interested in writing
Editorials, Articles, Reviews, and more. If you'd like to contribute
content, let us know.