Segmentation Fault at fclose

[programmer321]

Hello,
In one of my C codes, I have written a function for logging into a files. When the size of the file exceeds 10 MB, I close that file and open a new file. But at run time my Code is crashing at times during file closing. This behaviour is not at the same point each time.   at times the code is crashing, at times it is not.
I am attaching the stack trace for reference:
(gdb) bt full
#0  0x003187a2 in _dl_sysinfo_int80 () from /lib/ld-linux.so.2
No symbol table info available.
#1  0x0035c955 in raise () from /lib/tls/libc.so.6
No symbol table info available.
#2  0x0035e319 in abort () from /lib/tls/libc.so.6
No symbol table info available.
#3  0x0038ff8a in __libc_message () from /lib/tls/libc.so.6
No symbol table info available.
#4  0x0039694c in _int_free () from /lib/tls/libc.so.6
No symbol table info available.
#5  0x00396aca in free () from /lib/tls/libc.so.6
No symbol table info available.
#6  0x0038731a in fclose@@GLIBC_2.1 () from /lib/tls/libc.so.6
No symbol table info available.
#7  0x0805218a in printandlog (index=0, msg_str=0xbfefeb00 "sr Event 2082 292 a77c810 16") at main.c:3239
        Time = (struct tm *) 0x45d9e0
        Timet = 1205477277
        Timeb = {time = 1205477277, millitm = 925, timezone = -330, dstflag = 0}
        TimeStr = "[03/14 12:17:57.925 ] ", '\0' <repeats 64 times>
        buffer = "[03/14 12:17:57.925 ] sr Event 2082 292 a77c810 16\r\n", '\0' <repeats 934 times>
        l_tempbuf = "\000\000"
        l_totChar = 65
        num = 0
#8  0x0804d213 in Events () at main.c:1221
        channel = 16
        rc = 0
        code = 2082
     ..........................

The code of function is:
void printandlog(int index,char *msg_str)
{
        struct tm               *Time;
        time_t                  Timet;
        struct timeb            Timeb;
        char                    TimeStr[MAX_STRING_SIZE] = {'\0',};  /* will hold the time string when built    */
        char                    buffer[MAX_STRING_SIZE] = {'\0',};   /* will have everything but the time       */
        char l_tempbuf[3] = {'\0',};
        int l_totChar           =       0;
        int num                 =       0;

        //get_time(TimeStr);
        time(&Timet);
        Time = localtime(&Timet);
        ftime(&Timeb);                                               /* needed to get the milliseconds */
        sprintf(TimeStr,"[%02d/%02d %02d:%02d:%02d.%03d ] [%d] ",Time->tm_mon+1,Time->tm_mday,Time->tm_hour,Time->tm_min,Time->tm_sec, Timeb.millitm,index);
        strcpy(buffer, TimeStr);
        strcat(buffer, msg_str);
        strcat(buffer,"\n");
         if(fileflg == 0)
          {
                        printf("%s,%d",buffer,strlen(buffer));
                        l_totChar = fprintf(fp,buffer);
                        g_maxLogFileSize = g_maxLogFileSize + l_totChar;
                        if(g_maxLogFileSize >= MAXLOGSIZE)
                        {
                                //Closing preview file and creating new file
                                fclose(fp);
                                sprintf(l_tempbuf,"%c",g_logFile[strlen(g_logFilePath)+7]);
                                num = atoi(l_tempbuf);
                                if(num == MAX_LOG_FILES)
                                        num =1;
                                else
                                        num = num+1;
                                memset(g_logFile,'\0',sizeof(g_logFile));
                                sprintf(g_logFile,"%scmrlog_%d.txt",g_logFilePath,num);
                                printf("[%s]\n",g_logFile);

                                remove(g_logFile);
                                if((fp = fopen(g_logFile,"a+|w+")) == NULL)
                                        printf("Cannot open Log File For Writing.\n");
                                g_maxLogFileSize = 0;

                        }
                        fflush(fp);
                }
                else
                        printf("%s",buffer);
      
}

Can any one help me with this? Thanks in advance..

Regards

[jephthah]

it appears you're trying to close a file pointer before it has been opened.

[programmer321]

Hey I just forgot to specify that i am initially opening a file pointer before calling this function. Basicaaly, I am trying to implement cyclic logging i.e. if the my file size increeasies say 10 MB I close that file pointer n open the next. And after 5 files I open the first file again. This is running fine initially, but giving segmentation fault after some time. May be after writing to a couple of files.

[jephthah]

so you're using global file pointers and accessing them inside a subroutine?

that's a bad idea. if you open a file outside a subroutine and then need to access it inside a subroutine, you should pass the file pointer to the subroutine.

otherwise it's hard to tell whats going on because you've only showed your sub

[programmer321]

Thanks for ur response.. Well I do understand its difficult to understand from a code snippet. I ll try to explain wat I am trying to do. I hv taken a global file pointer. I open a file wit that file pointer vry initially in my program. And for every statement which I want to log into that file, I call my this particular subroutine(printandlog). Inside this subroutine, I chk if the size of file exceeds 10MB I close that file pointer and open a new file using that file pointer.
As per ur response, I am not able to understand whether accessing the global pointer directly inside the subroutine or passing it as an argument, will it actually make any diiference(other then increasing theunderstandability of the code).
Pls advice.

[jephthah]

my suspicion is that you are losing track of how many times you have opened / closed the file pointer, and that you are trying to read a pointer that is not opened.or close a pointer that is not opened.

this would explain why you're getting a segmentation fault. this is a common error that happens when you start using globals inside subroutines, it's just generally bad practice for that reason.

of course i could be wrong .... buti don't have anything else to go on, since you've only given subroutine.

it definitely is very suspicious though. i would rewrite the program to not use global file pointers.

[Petrock6]

Try setting a very large new buffer.

[jephthah]

Try setting a very large new buffer.

yah, that, or maybe just call a flea flicker and have the running back throw a hail mary to the kicker in the end zone.

[mitrmkar]

Try valgrind
http://valgrind.org/info/

As a general note, you should add error checking to your code. As of now, there is none.

[programmer321]

Hello,

Thanks everybody, the issue is resolve. The problem was - the same logging function was being called by two threads without any mutex or concurrency checks being appiled on the functions. Primarily, mitrmkar's suggestion of putting error checking helped me resolving the issue.

Regards.

[jephthah]

global file pointers in a multithreaded environment? no wonder you've got problems. To repeat what i've been saying... this underscores the reason why you shouldnt use global file pointers. you can't keep track of them worth a damn, and god help the person who has to maintain your code a year or two from now.

globals, sparingly used, do have their place ... but they should not be used as a relief for poorly specified functions.

try spec'ing your functions more thoroughly and pass your file pointers as arguments.

[programmer321]

Definitely Jephthah. I will take definitely work upon ur advice. Thanks.

Regards.