3

The following code is from a hackerrank question Here we have a code entering n words and then it inputs q queries with each query consisting of a word itself, here output corresponds to the number of occurences the word in query had in the original array. As i have asked in the question title and comment , why does it not work with malloc(for bigger values) and work with non- dynamic allocatiion??

#include<stdio.h>
#include<stdlib.h>
#include<string.h>
int main(){
    int n,q,count;
    char temp[20];
    scanf("%d",&n);
    char **a=(char **)malloc(n*20*sizeof(char));//malloc fails here,But works for char a[n][20];
    for(int i=0;i<n;i++){                       //WHY do i get garbage value with malloc for bigger values??
        scanf("%s",a+i);
    }
    scanf("%d",&q);
    while(q--){
        count=0;
        scanf("%s",temp);
        for(int i=0;i<n;i++) if(strcmp(a+i,temp)==0) count++;
        printf("%d\n",count);
    }
    return 0;
}
Neel
  • 33
  • 4
  • What is the input value for `n`? – Sourav Ghosh Mar 27 '20 at 07:31
  • n =<1000 plz see the link in the question for other constraints @SouravGhosh – Neel Mar 27 '20 at 07:34
  • Because you cannot scanf input into uninitialized pointers... you need to allocate memory for the actual data, not just the pointers. – Lundin Mar 27 '20 at 07:35
  • 1
    You are declaring `n * 20 * sizeof(a_pointer)` ***pointers***. – David C. Rankin Mar 27 '20 at 07:35
  • What is the rationale behind `char **a=(char **)malloc(n*20*sizeof(int*));`? Especially what is the `sizeof(int*)` for? Did you run your program in a debugger and look at the values you get for `a` and `a+i` and the content of the malloced memory during the first `for` loop? – Werner Henze Mar 27 '20 at 07:35
  • You have one allocation in this program, a bunch of pointers. They don't point to *anything* determinate. Unrelated, what that `int*` is doing in your size computation is a mystery. If anything it should be `char*`, and although the resulting side will be the same, that won't save this. You need more than a pointer per item; you need *memory* each of those pointers can point to. – WhozCraig Mar 27 '20 at 07:35
  • 1
    You can declare `char (*a)[20] = malloc (n * sizeof *a);` to declare storage `n` 20-character blocks all in one allocation which you can index as a 2D array (and have the benefit of a single `free()`) Here your type is *pointer-to-array-of char [20]*. – David C. Rankin Mar 27 '20 at 07:38
  • The basic problem with your allocation is you allocate for `char**` which is a single pointer -- to what? (answer: a block of memory for **pointers**). So when you attempt `a + i` you advance `a_pointer` number of bytes each time (generally 8 on x86_64 or 4 on x86). If you want to go that route, then you must allocate `n` pointers, and then allocate storage for each 20-character block and assign the beginning address for each block to your pointers in sequence. Or as an alternative, see my earlier comment on use of a pointer-to-array-of `char [20]`. In either case, you use `a[i]` not `a+i`. – David C. Rankin Mar 27 '20 at 08:03

1 Answers1

4

The basic problem with your allocation is you allocate for char** which is a single pointer to what? (a block of memory holding pointers) So when you attempt a + i you advance a_pointer number of bytes each time (generally 8 on x86_64 or 4 on x86).

If you want to use a pointer-to-pointer-to char (commonly called a double-pointer), then you must allocate storage for n pointers, and then allocate storage for each 20-character block you need and assign the beginning address for each block to your pointers in sequence. You can then access each allocated block with a[i] where 0 <= i < n where the beginning of each string is 8-bytes on x86_64 away from the previous pointer. (there is no guarantee that the storage for the strings will be contiguous in memory, but the pointers will)

Or as an alternative, you can use a pointer-to-array-of char [20]. (e.g. char (*a)[20];) which will allow you to allocate for n 20-character arrays in a single allocation. There your type is pointer-to-array-of char [20] so the beginning of each string a[i] is 20-bytes after the previous string a[i-1] and all 20-character blocks of storage a guaranteed to be sequential in memory. As an added benefit with the pointer-to-array, since there is only a single-allocation, there is only a single free() required.

Both approaches allow you to index each character as you would in a 2D array. (e.g. a[i][j] where 0 <= i < n and 0 <= j < 20.

By far the approach using the pointer-to-array is the most convenient if you are allocating blocks of the same size. Where using the pointer-to-pointer provides a benefit is if you allocate only as much storage as is required by each string. (though it does add a bit of coding complexity to do correctly)

To allocate using the pointer-to-array you simply need:

    char (*a)[20] = malloc (n * sizeof *a); /* allocate for n 20-char blocks */
    if (!a) {       /* validate EVERY allocation */
        perror ("malloc-a");
        return 1;
    }

(note: in C, there is no need to cast the return of malloc, it is unnecessary. See: Do I cast the result of malloc?)

Also note, you must validate every allocation by checking the return, just as you must validate every user-input by checking the return (If you take nothing else from this answer, learn those two concepts, otherwise you invite Undefined Behavior in your code).

With that change, your example becomes:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main (void) {

    int n, q;
    char temp[20];

    if (scanf ("%d", &n) != 1) {    /* validate EVERY input */
        fputs ("error: invalid integer value 'n'.\n", stderr);
        return 1;
    }

    char (*a)[20] = malloc (n * sizeof *a); /* allocate for n 20-char blocks */
    if (!a) {       /* validate EVERY allocation */
        perror ("malloc-a");
        return 1;
    }

    for (int i = 0; i < n; i++)
        if (scanf ("%19s", a[i]) != 1) {    /* VALIDATE */
            fprintf (stderr, "error: failed reading word '%d'\n", i);
            return 1;
        }

    if (scanf ("%d", &q) != 1) {            /* VALIDATE */
        fputs ("error: invalid integer value 'q'.\n", stderr);
        return 1;
    }
    while (q--) {
        int count = 0;
        if (scanf ("%19s", temp) != 1) {    /* VALIDATE */
            fputs ("error: reading 'temp'.\n", stderr);
            return 1;
        }

        for (int i = 0; i < n; i++)
            if (strcmp (a[i], temp) == 0)
                count++;

        printf ("%d word(s) match '%s'\n", count, temp);
    }

    free (a);       /* free allocated memory */
}

(note: every user-input and allocation is validated and every scanf() conversion specifier for a string uses the field-width modifier to protect the allocated bounds for each block of memory)

Example Use/Output

$ ./bin/dynptr2arr
10
my dog has fleas my cat has none happy cat
3
dog
1 word(s) match 'dog'
has
2 word(s) match 'has'
fleas
1 word(s) match 'fleas'

Memory Use/Error Check

In any code you write that dynamically allocates memory, you have 2 responsibilities regarding any block of memory allocated: (1) always preserve a pointer to the starting address for the block of memory so, (2) it can be freed when it is no longer needed.

It is imperative that you use a memory error checking program to ensure you do not attempt to access memory or write beyond/outside the bounds of your allocated block, attempt to read or base a conditional jump on an uninitialized value, and finally, to confirm that you free all the memory you have allocated.

For Linux valgrind is the normal choice. There are similar memory checkers for every platform. They are all simple to use, just run your program through it.

$ valgrind ./bin/dynptr2arr
==19181== Memcheck, a memory error detector
==19181== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==19181== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==19181== Command: ./bin/dynptr2arr
==19181==
10
my dog has fleas my cat has none happy cat
3
dog
1 word(s) match 'dog'
has
2 word(s) match 'has'
fleas
1 word(s) match 'fleas'
==19181==
==19181== HEAP SUMMARY:
==19181==     in use at exit: 0 bytes in 0 blocks
==19181==   total heap usage: 3 allocs, 3 frees, 2,248 bytes allocated
==19181==
==19181== All heap blocks were freed -- no leaks are possible
==19181==
==19181== For counts of detected and suppressed errors, rerun with: -v
==19181== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Always confirm that you have freed all memory you have allocated and that there are no memory errors.

Look things over and let me know if you have further questions.

David C. Rankin
  • 81,885
  • 6
  • 58
  • 85
  • Thanks for answering.On hackerrank when i input my given code , it still works for some values, can u tell me why that may be?? – Neel Mar 27 '20 at 10:42
  • Yes, hackerrank is all about testing every ***corner-case***. That means all strings of `"aaaaaaaaaaaaaaaaa"`, `"abbabba"`, etc.. or all empty strings `""`. Pay close attention to the requirements and think though all the corner cases you can -- and you still might end up spending a few hackos on testcases... – David C. Rankin Mar 27 '20 at 22:20