0

the task is to dynamically allocate memory for the array of structures and then fill them from keyboard. I was able to dynamically allocate and fill amount of pages for each of the struct instance in array, but when I try to add char* to it by doing something like: strcpy(myArray[i]->author, authorName);

But every time I get segmentation error, so what am I doing wrong? Is it possible that problem is actually in memory allocation?

Here is the code

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

struct Book {
    char* author;
    char* title;
    int pages;
    int pubYear;
    int copies;
};

void allocList(struct Book **myArray, int booksAmount);
void fillKeyboard(struct Book **myArray, int booksAmount);

int main(void) {

    struct Book *booksList = NULL;

    int booksAmount = 3;

    allocList(&booksList, booksAmount);

    fillKeyboard(&booksList, booksAmount);

  return 0;
}

void allocList(struct Book **myArray, int booksAmount) {
    *myArray = (struct Book*) malloc(sizeof(struct Book) * 100);
    printf("memory for %d books was allocated \n", booksAmount);
}

void fillKeyboard(struct Book **myArray, int booksAmount) {

    int i = 0;

    char* authorName = "author name";

    while (booksAmount--) {
        printf("book number %d \n", i + 1);
        printf("enter amount of pages: ");
        scanf("%d", &(*myArray)[i].pages);
        printf("\nenter author: ");
        strcpy(myArray[i]->author, authorName);
        printf("%s is \n", authorName);
        i++;
        printf("\n");
    }

}

Thank you.

ChrisMM
  • 8,448
  • 13
  • 29
  • 48
Serghey Hmeli
  • 63
  • 1
  • 6
  • 4
    `c` and `c++` are different languages with different concepts, and the answer would be different depending on that. So remove one of those tags to make clear which language you use. Based on the code you are asking a question about `c` and not `c++` – t.niese Dec 23 '19 at 14:36
  • 1
    [Please see this discussion on why not to cast the return value of malloc() and family in C..](https://stackoverflow.com/q/605845/2173917) – Sourav Ghosh Dec 23 '19 at 14:37
  • `strcpy(myArray[i]->author, authorName);` is most likely to be the culprit here. – Sourav Ghosh Dec 23 '19 at 14:39
  • Note that code never ask for a new `authorName`. – chux - Reinstate Monica Dec 23 '19 at 15:01
  • 1
    You have to allocate memory for the `author` _before calling_ `strcpy`. That is: `(*myArray)[i].author = malloc(MAX_AUTHOR_NAME_LENGTH+1);`. Also arguments of your `strcpy` call are incorrect. It must be `strcpy((*myArray)[i].author, authorName);`. Your extra indirection of the parameter `myArray` in the function `fillKeyboard` complicates the code unnecessarily. – Lxer Lx Dec 23 '19 at 16:03
  • @chux-ReinstateMonica Yes, it is not yet. But I will add this as soon as I figure out the reason of segmentation fault. Thank you – Serghey Hmeli Dec 23 '19 at 19:05
  • @LxerLx Thanks, now it is working as intended – Serghey Hmeli Dec 23 '19 at 19:41

2 Answers2

3

myArray[i].author is a string. so (in C) an array of char. as an array you need to allocate it with a malloc

 myArray[i].author=malloc(sizeof(char) * 100);

your while loop should look like this:

 while (booksAmount--) {
    myArray[i].author=malloc(sizeof(char) * 100);
    printf("book number %d \n", i + 1);
    printf("enter amount of pages: ");
    scanf("%d", &(myArray)[i].pages);
    printf("\nenter author: ");
    strcpy(myArray[i].author, authorName);
    printf("%s is \n", authorName);
    i++;
    printf("\n");
}

keep in mind that 100 is a "magic number" so if the author name is longer than 100 character it's not going to work

edit: same thing with the title, you need to allocate the needed memory in your array's element

lory9894
  • 63
  • 5
  • 1
    Off by 1, "if the author name is longer than 99". IAC consider `malloc(strlen(authorNam)+1);` if `authorNam` known before allocation. – chux - Reinstate Monica Dec 23 '19 at 14:59
  • That code is incorrect. The expression `myArray[i].author` makes no sense. It must be changed into `(*myArray)[i].author`. The original poster's extra indirection on the parameter `myArray` makes the code complicated unnecessarily. – Lxer Lx Dec 23 '19 at 16:17
1

If you care about memory, you should not create an array of 100 books. A good way to avoid such allocations is linked lists, so your program will always allocate just the needed memory. Your Book struct would look like this:

struct Book {
    char* author;
    char* title;
    int pages;
    int pubYear;
    int copies;
    struct Book *next;
};

The program will certainly be a bit more complex, but the result is a clean memory usage, and a truly dynamic program.

Lucas Gras
  • 961
  • 1
  • 7
  • 22