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

struct bank
{
    char *name [3]; 
    char *ha[3];
    int bal[3];
};

typedef   struct bank   bank;

int main()
{
    bank *SBI; 

    SBI=(bank*) malloc(sizeof(bank));

    strcpy(SBI->ha[0], "1234"); 

    printf("SUCCESS");

    return 0;
}

Why is the above code generating a memory write error? When I run the code it generates some error relating to memory. I am beginner in C programming. Can anyone help me about what is wrong in the code that is causing the error.

GSerg
  • 76,472
  • 17
  • 159
  • 346
  • Ok I will edit it now – World Producer Dec 11 '21 at 13:43
  • you need to allocate memory for `SBI->ha[0]` – Ôrel Dec 11 '21 at 13:46
  • Thank you Ôrel . Your solution really worked out but I didn't understand one thing . Whenever I declare a character array of pointer in main() I don't have to allocate memory for it but Why do I have to allocate memory when I declare it in a structure ? – World Producer Dec 11 '21 at 13:55
  • @WorldProducer You *always* have to take care of memory allocation for your pointers. An uninitialized pointer points nowhere good; you have to initialize each pointer either by calling `malloc`, or by assigning it the address of some object (typically an array) that the compiler has allocated for you. If you're just starting out with pointers, it will take a while to learn all this -- memory allocation is what makes pointers hard in C. – Steve Summit Dec 11 '21 at 14:26
  • Also I'm suspicious of declarations like `char *name [3];`. That says you're going to have *three* names — although you haven't allocated memory for any of them yet. Is that what you meant? If you just wanted one name, and if you wanted to let the compiler take care of the memory allocation for now, you could have used something like `char name[30];`. That would get you one name, of up to 29 characters (plus the terminating null character). – Steve Summit Dec 11 '21 at 14:26

2 Answers2

1

You also need to allocate space for your three strings ha[0], ha[1], ha[2]. Your malloc allocates memory for the bank structure, including three pointers, but these pointers need to be allocated too, with malloc or strdup for example:

for (int i = 0; i < 3; i++) {
    bank->ha[i] = malloc(MY_STRING_MAXLENGTH);
}

which you later free:

for (int i = 0; i < 3; i++) {
    free(bank->ha[i]);
}
free(SBI);
mmixLinus
  • 1,646
  • 2
  • 11
  • 16
  • 1
    Please don't cast malloc result – Ôrel Dec 11 '21 at 13:47
  • @orel, why do you think that casting malloc result is wrong? – shrewmouse Dec 11 '21 at 13:51
  • It can hide an error.https://stackoverflow.com/a/605858/4605105 – Ôrel Dec 11 '21 at 13:53
  • @Ôrel the author of the accepted answer there accepts that the premises are partially wrong: Some compilers WILL FAIL you for not having the right return type. (See the comments) But sure, I will remove the cast – mmixLinus Dec 11 '21 at 14:08
  • 1
    @mmixLinus The custom of *not* casting `malloc` in C is not as strong as it once was. The error that it used to hide is much less common (and, arguably, impossible) today. However, if a compiler "fails you" in this case, it means you're trying to use a C++ compiler on C code, which is a bad idea for a bunch of other reasons. – Steve Summit Dec 11 '21 at 14:23
  • I think type casting malloc result to the specific data type is not such a bad thing because as Steve Summit said that the hidden error is much less common in most C compilers – World Producer Dec 12 '21 at 09:13
0

You are copying your string into unallocated memory.

strcpy(SBI->ha[0], "1234")

Use strdup instead of strcpy. Strdup will allocate the memory for you.

shrewmouse
  • 5,338
  • 3
  • 38
  • 43