0

I have a really quick question. Why do i get heap corruption detected when i try to deallocate the array in the void translateWord() function? I tried to deallocate line by line in a for loop but it doesnt seem to work. I thought that if i use that function more than once, and every time the function allocates memory, i should deallocate it at the end of the function. Any idea?

#define _CRT_SECURE_NO_WARNINGS
#include<stdio.h>
#include <conio.h>
#include <string.h>
#include <stdlib.h>
#include <ctype.h>


void translateWord(char word[], FILE *point_out, FILE *point_1)
{
    rewind(point_1);
    char ch;
    int gasit = 0;
    int lines = count_lines(point_1);
    int i = 0;
    char *cp;
    char *bp;
    char line[255];
    char **array = (char**)malloc(sizeof(char*)*2*lines);

    rewind(point_1);
    while (fgets(line, sizeof(line), point_1) != NULL) {
        bp = line;
        while (1) {
            cp = strtok(bp, "=\n");
            bp = NULL;

            if (cp == NULL)
                break;
            array[i] = (char*)malloc(sizeof(char)*strlen(cp));
            strcpy(array[i++], cp);
        }
    }

    gasit = cuvant_gasit(word, array, lines, point_out, gasit);
    if (gasit == 0)
    {
        fprintf(point_out, "<<%s>>", word);
    }

    for (int k = 0; k < 2 * lines; k++)
    {
        free(array[k]);
    }
    free(array);
}
valentin.ciprian
  • 91
  • 1
  • 9
  • did you executed it with _valgrind_ to see where you have an invalid memory modification or something like ? – bruno Jan 02 '19 at 17:28
  • @bruno, i dont know what is valgrind or how to use it. I just started learning C two months ago – valentin.ciprian Jan 02 '19 at 17:32
  • It is difficult for us without an example of "rom-eng.txt" input file producing the problem. – bruno Jan 02 '19 at 17:32
  • @valentinciprianyou don't know _valgrind_ ? it is not forbidden to search about it ;-) _valgrind_ is really useful, look at it – bruno Jan 02 '19 at 17:33
  • 1
    Welcome to SO! Please post a [mcve] which contains the text file contents and the smallest amount of code that reproduces the error. – ggorlen Jan 02 '19 at 17:33
  • @bruno, okk, i`ll look into it :D – valentin.ciprian Jan 02 '19 at 17:37
  • @ggorlen, well it says that the application wrote to memory after the of heap buffer – valentin.ciprian Jan 02 '19 at 17:38
  • @bruno, that text file i wrote on every line something like this: word=wordtranslated. But that part of the code works fine. Well i could say the code works without the need to deallocate the memory, but from what i here that is "bad practice" – valentin.ciprian Jan 02 '19 at 17:41
  • The code looks Windows-ish to me, so I don't know if valgrind is an option, but [there are other tools for Windows](https://stackoverflow.com/q/413477/10077). BTW, [your `main` signature is wrong](https://stackoverflow.com/q/2108192/10077). – Fred Larson Jan 02 '19 at 17:42
  • @valentin.ciprian I see a strcpy going out of the allocated block of memory, look at my answer and replace the two lines with the simple _strdup_ then try again – bruno Jan 02 '19 at 17:43
  • @valentin.ciprian: What "minimal" means, among other things, is this: Rip out all the parts that do not **directly** contribute to the problem. An example for your problem -- how to deallocate memory -- would ideally *only* include the allocation and deallocation of memory, with all the irrelevant parts, those that matter only in the context of your complete program, taken out / replaced with "stuff happens here" comments. The *minimal* amount of code required to reproduce, not the functionality of your program, but *your problem*. This "reduction of scope" is an important debugging technique. – DevSolar Jan 02 '19 at 17:45
  • @DevSolar, sorry for the inconvenience. Its my second time posting here. Next time i will be more carefull. – valentin.ciprian Jan 02 '19 at 17:47
  • (ctd.) When this hasn't happened, and "the whole program" is being posted (like in your case), this might get interpreted as, "you haven't bothered trying to find the problem yourself". Because if you had tried, you would already *have* a smaller example. We react indulgent if this happens to a new contributor, but it's pretty much a requirement on SO to do this kind of "scope reduction" if you don't want to be met with the downvote / close-vote hammer. (Partly because, usually, there are many, many *other* problems in a non-minimal example that would *also* have to be addressed...) ;-) – DevSolar Jan 02 '19 at 17:47
  • Also see [**Why is “while ( !feof (file) )” always wrong?**](https://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong) – Andrew Henle Jan 02 '19 at 17:48
  • Calling `cp = strtok(bp, "=\n");` within a loop is an incorrect use of `strtok`, the first call uses `bp`, all subsequent calls use `NULL`. You must limit `while (1)` to execute no more than `2*lines` times (or you run out of pointers) – David C. Rankin Jan 02 '19 at 17:54
  • @DavidC.Rankin, well after the first call, if i write bp=NULL, its not ok at all? – valentin.ciprian Jan 02 '19 at 17:57
  • @AndrewHenle, i`ve just seen that question a few moments ago. I`ll read the answers. – valentin.ciprian Jan 02 '19 at 18:23
  • You don't write `bp = NULL` you write `cp = strtok(NULL, "=\n");` look at [man strtok](http://man7.org/linux/man-pages/man3/strtok.3.html) `"On the first call to strtok(), the string to be parsed should be specified in str. In each subsequent call that should parse the same string, str must be NULL."` – David C. Rankin Jan 02 '19 at 19:08

1 Answers1

3

There is something wrong in translateWord :

array[i] = (char*)malloc(sizeof(char)*strlen(cp));
strcpy(array[i++], cp);

The first line must be array[i] = (char*)malloc(strlen(cp) + 1); else 1 char is missing to save the final null char during the strcpy.

Note that by definition sizeof(char) is 1.

And why do you not just use strdup rather than a malloc then a strcpy ? just replace these 2 lines by array[i++] = strdup(cp);

bruno
  • 32,421
  • 7
  • 25
  • 37
  • wow, i really forgot that plus 1. Sorry for not paying enough attention. Now works. I will look into strdup, Till now i didnt use something like this – valentin.ciprian Jan 02 '19 at 17:45
  • 1
    `strlen()`, `malloc()` and `strcpy()` are language standard, `strdup()` -- while being widely available -- isn't. Depending on environment, this might make `strdup()` a non-option. – DevSolar Jan 02 '19 at 17:50
  • @DevSolar in that case to define _strdup_ to be able to reuse it everywhere when needed is not really just an option and is almost mandatory :-) – bruno Jan 02 '19 at 17:54
  • @bruno: Others dislike `strdup()` because it's hiding a dynamic memory allocation behind a function name that isn't `malloc()`. ;-) Tastes differ. – DevSolar Jan 02 '19 at 17:55
  • @DevSolar say them it is never too late to learn and memorize something new ;-) – bruno Jan 02 '19 at 17:58
  • @bruno, i`ve just read a little about strdup(), and i think about using it :) thanks for the information – valentin.ciprian Jan 02 '19 at 18:03
  • Please don't cast malloc in an answer. It's a bad habbit. – klutt Jan 03 '19 at 02:09
  • @Broman what a curious remark, personally I think exactly the reverse. When I can I compile C codes both with gcc and g++, so the cast is mandatory to avoid error in C++. – bruno Jan 03 '19 at 07:41