Random Error in C

Question

I'm a newbie in C programming. I wrote a program which renames files in series. Sometimes it works, sometimes -in same conditions - it crashes without no apparent reason. The code error it gives is c000005, i searched and discovered that it is a problem with the memory. I noticed that if i run the program from the IDE (Code::Blocks 13.12) it always works! But when i launch the app from the debug directory or from the release one, sometimes it crashes! How could i solve this problem?

The program is meant to rename subtitles of TV episodes. The subtitles will be named in order: if the season is number two, the first episode of the second season will be 2x01 - EpisodeTitle.srt, the second 2x02 - Episode Title.srt, and so on. Many times in the subtitles there is written the name of the episode, so the program will search for it in the .srt file. If it finds the "Episod" string, than it searches for the string between two inverted commas ("). So for example, if in the subtitle file there is this kind of line:

Episodio 01 "Chapter 1"

The program copy the title Chapter 1 and insert it in the name of the file, so it will be renamed 1x01 - Chapter 1.srt

If the program doesn't find the title, situated between two inverted commas, it will search for it in the line below. So if in the file there are two lines like those, the program will still work:

Episodio 01:

"Chapter 1"

If there isn't the name of the episode inside the subtitles, the program will name the episode "1x01.srt" (if it is the first episode of the first season")

#include <stdio.h>
#include <sys/stat.h>
#include <dirent.h>
#include <limits.h>
#include <string.h>
#include <stdlib.h>
void f_rename(char stg);
char *FindName(char* file);
char stg;

int main()
{
    printf("Che stagione?"); /* Asks for the number of the season*/
    scanf("%c",&stg);
    f_rename(stg);
}


void f_rename(char stg)
{
DIR *d;
struct dirent *ent;
char *newname=(char*)malloc(sizeof(char)*9);
newname[0]=stg; /* sets the first character to the number of the season */
newname[1]='x'; 
d= opendir(".");
int i=0;int k=0; int count1=0;int count2=0;
if (d)
{
while ((ent = readdir(d)) != NULL) /* ent struct contains the name of the file (d_name) */
    {
    if (strcmp(ent->d_name, ".") != 0 && strcmp(ent->d_name, "..") != 0) /*delete . and .. entry of readdir()*/
        {
        if (strstr(ent->d_name, ".srt") != NULL) /* searches only for .srt*/
            {
            i++; /*count how many .srt files there are*/
            char *result = FindName(ent->d_name);
            if (i==10) {count2++, i=0;} /* if there are 10 subtitles, it sets i to zero, in order to have 1x10*/
            newname[2] = (char)(count2 + (int)'0'); 
            newname[3] = (char)(i + (int)'0'); /*sets the number of the subtitles as a character*/
            if (result != NULL) /* Check if there is the name of the episode inside subtitles*/
                {
                int len = strlen(result);
                newname=(char*)realloc(newname,sizeof(char)*(len+11)); /*gives newname only the space it needs, so 7char ("1x01 - ") plus 4 (.srt) plus the lenght of the name of the episode)*/
                newname[4]=' ';newname[5]='-';newname[6]=' ';
                for (int j=7;j<len+4;j++) newname[j]='\0';
                strcat(newname, result);
                strcat(newname,strstr(ent->d_name, ".srt"));
                rename(ent->d_name,newname);
                }
            else /* if there isn't the name of the episode inside the subtitles, give the name 1x01.srt*/
                {
                newname=(char*)realloc(newname,sizeof(char)*9);
                for(int j=4;j<8;j++) newname[j]='\0';
                strcat(newname,strstr(ent->d_name, ".srt"));
                rename(ent->d_name,newname);
                }
            }
        }
    }
closedir(d);
}
}


char *FindName(char *file)
{
   char string[]="Episod";char *result;
   FILE *stream = fopen(file,"r");
   char buf[300];
   char *p1; char *p2; int count=0;int count2=0;
   while(fgets(buf, 300, stream) != NULL)
    {
/* if it found the string 'Episod', then searches for the inverted comma, if there isn't, it searches for it in the next line, if the there isn't, the function returns 0. */
            if ((strstr(buf,string)) != NULL || count2==1) /* searches the string 'Episod */
            {
            count++; 
            if (strstr(buf,"\"")==NULL) 
                {
                if (count==2) break;  
                else {count2++;continue;}
                }
            p1= strstr(buf,"\"")+1; /* take the position of the begin of the name*/
            p2= strstr(p1,"\"");
            size_t len = p2-p1;
            result=(char*)malloc(sizeof(char)*(len+1));
            strncpy(result,p1,len); /* copy the name into result*/
            result[len]='\0';
            fclose(stream);
            return(result); 
            }
        }
        fclose(stream);
        return(0); /*if it didn't find the name, return 0 */
    }

I tried to debug and it reports a problem in TpWaitForAlpcCompletion(), i don't know what does it mean. Is there anyone who could help me in order not to make it crash?

This looks like a *stellar* time to brush up on using a debugger. Btw, there's this compliment to `malloc` and `realloc` called `free`. And [**stop casting `malloc` in C programs**](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) — WhozCraig, Oct 05 '14 at 17:51
*Debug your program*. This code has numerous questionable activities. And your question is not specific. "Sometimes this crashes" isn't a question; its a *problem*. As-posted this code will not even compile, as you've omitted all (if any) `#include` headers. Nor have you included examples of the input directory filenames, content of files that are being cracked open by `FindName`, and any debugging efforts so far. This is intended to perform a series rename, yet leave it to use to decipher what that actually *means*. You need to put more effort into this before someone else will. — WhozCraig, Oct 05 '14 at 18:19
Your fundamental problem is that you don't know what your program is doing. You have no moral right to expect it to work while that remains true. You should always know what your program is doing, and if you find that you've written one without having this knowledge, a debugger will help you rectify the situation. — Crowman, Oct 05 '14 at 20:01
I explained what the program does as much as i could. Hope this is enough — Carlo Cepollaro, Oct 05 '14 at 20:22
Hint: You could use sprintf() or snprintf() to construct the new name. — wildplasser, Oct 05 '14 at 20:52

score 1 · Accepted Answer · answered Oct 05 '14 at 23:33

Your code is very labyrinthine and difficult to follow, which is part of your problem. All this count and count2 stuff isn't explanatory of what you want to do. If you write programs in a simpler way you'll find them easier to debug.

You're also missing all kinds of checks you should be making, including the return values from fopen(), rename(), malloc(), realloc(), to name a few.

Your string handling is very laborious and unnecessary. Your comments make your code harder to follow, not easier. There are steps, casts, sizeof(char)s, and memory allocations which are just outright unnecessary, and make your code harder to follow. You inexplicably use a global variable to store a character which you then pass by value to a function and never use again.

The theme here is that there are ways of writing your programs which will make things a lot easier for you when they go wrong. In addition, learning to use a debugger and a good memory debugging tool like Valgrind is always worth the effort.

To even start working with your program I had to essentially rewrite it to have much of a fighting chance of figuring out what you were doing, because that's quicker and easier than following the code you had. Any time something works sometimes, and doesn't work at other times, it's often anyone's guess, so the best answer is "don't write your programs like this". Most likely you are writing to memory you shouldn't be writing to and it sometimes causes problems. Undefined behavior is like that.

Here's a tidied-up version of your code for you to compare with your own:

#define _POSIX_C_SOURCE 200809L

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdbool.h>
#include <limits.h>
#include <sys/stat.h>
#include <dirent.h>

#define MAX_LINE 1024

void rename_files(const char season);
char * find_episode_name(const char * filename);

int main(void)
{
    printf("Enter the season number: ");
    fflush(stdout);

    char buffer[MAX_LINE];
    if ( !fgets(buffer, MAX_LINE, stdin) ) {
        fprintf(stderr, "error calling fgets()");
        exit(EXIT_FAILURE);
    }
    else if ( !buffer[0] ) {
        fprintf(stderr, "You entered no input.\n");
        exit(EXIT_FAILURE);
    }

    rename_files(buffer[0]);

    return EXIT_SUCCESS;
}

void rename_files(const char season)
{
    DIR * d;

    if ( (d = opendir(".")) ) {
        int num_srt = 0;
        struct dirent * ent;

        while ( (ent = readdir(d)) ) {
            if ( strcmp(ent->d_name, ".") &&
                 strcmp(ent->d_name, "..") ) {
                if ( strstr(ent->d_name, ".srt" ) ) {
                    ++num_srt;

                    char newname[MAX_LINE];
                    char * result = find_episode_name(ent->d_name);
                    if ( result ) {
                        snprintf(newname, MAX_LINE, "%cx%02d - %s.srt",
                                 season, num_srt, result);
                    } else {
                        snprintf(newname, MAX_LINE, "%cx%02d.srt",
                                 season, num_srt);
                    }

                    if ( rename(ent->d_name, newname) == -1 ) {
                        char buffer[MAX_LINE];
                        snprintf(buffer, MAX_LINE,
                                 "error renaming file '%s'", ent->d_name);
                        perror(buffer);
                        exit(EXIT_FAILURE);
                    }

                    free(result);
                }
            }
        }

        if ( closedir(d) == -1 ) {
            perror("error closing directory");
            exit(EXIT_FAILURE);
        }
    }
}

char * find_episode_name(const char * filename)
{
    const char * epi = "Episod";
    char * result = NULL;
    char buffer[MAX_LINE];

    FILE * infile = fopen(filename, "r");
    if ( !infile ) {
        snprintf(buffer, MAX_LINE, "error opening file '%s'", filename);
        perror(buffer);
        exit(EXIT_FAILURE);
    }

    bool keep_looking = true;
    bool next_line = false;

    while ( keep_looking && fgets(buffer, MAX_LINE, infile) ) {
        if ( strstr(buffer, epi) || next_line ) {
            const char * p1 = strchr(buffer, '"');

            if ( !p1 ) {
                if ( !next_line ) {
                    next_line = true;
                }
                else {
                    keep_looking = false;
                }
            }
            else {
                ++p1;
                const char * p2 = strchr(p1, '"');
                if ( !p2 ) {
                    fprintf(stderr, "Badly formed input in file %s\n",
                            filename);
                    exit(EXIT_FAILURE);
                }

                const size_t len = p2 - p1;
                if ( !(result = malloc(len + 1)) ) {
                    perror("couldn't allocate memory for result");
                    exit(EXIT_FAILURE);
                }

                strncpy(result, p1, len);
                result[len] = '\0';
                keep_looking = false;
            }
        }
    }

    if ( fclose(infile) == EOF ) {
        snprintf(buffer, MAX_LINE, "error closing file '%s'", filename);
        perror(buffer);
        exit(EXIT_FAILURE);
    }

    return result;
}

and some sample output:

paul@thoth:~/src/sandbox/soq$ ls
1x01 - Episode 1.srt  1x04 - Random Name.srt          soq
1x02 - Episode 2.srt  1x05 - Another Random Name.srt  soq.c
1x03 - Episode 3.srt  data
paul@thoth:~/src/sandbox/soq$ cat '1x01 - Episode 1.srt'
paul@thoth:~/src/sandbox/soq$ cat '1x02 - Episode 2.srt'
Episodio 2
"Free ride on a donkey"
paul@thoth:~/src/sandbox/soq$ cat '1x03 - Episode 3.srt'
Episodio 3
"Milton Keynes is a long way"
paul@thoth:~/src/sandbox/soq$ cat '1x04 - Random Name.srt'
paul@thoth:~/src/sandbox/soq$ cat '1x05 - Another Random Name.srt'
Episodio 66 - "Fruitless in Milan"
paul@thoth:~/src/sandbox/soq$ ./soq
Enter the season number: 1
paul@thoth:~/src/sandbox/soq$ ls
1x01 - Fruitless in Milan.srt  1x04 - Free ride on a donkey.srt        soq
1x02.srt                       1x05 - Milton Keynes is a long way.srt  soq.c
1x03.srt                       data
paul@thoth:~/src/sandbox/soq$

As you'll see, your program relies on the order in which readdir() returns things to you to number your episodes, which may be what you want, but probably is not.

score 0 · Answer 2 · answered Oct 05 '14 at 21:47

One of the problems is I believe you have underallocated the space required for "newname" ... you have not accounted for the null-terminator.

Your code (with a bit of reformatting) says:

            int len = strlen(result);
            newname=(char*)realloc(newname,sizeof(char)*(len+11)); 
            /*
             * gives newname only the space it needs, so 7char ("1x01 - ") 
             * plus 4 (.srt) plus the lenght of the name of the episode)
             */

It should be:

            int len = strlen(result);
            newname=(char*)realloc(newname,sizeof(char)*(len+12)); 
            /*
             * gives newname only the space it needs, so 7char ("1x01 - ") 
             * plus 4 (.srt) plus the length of the name of the episode 
             * plus 1 for the null-terminator)
             */

Random Error in C

2 Answers2