CODEWITHSUNDEEP
cpointerssegmentation-faultmalloc
user641687
user641687

Reputation:

Segmentation fault in C program using array of char * and malloc

Silly little C program, but for the life of me I can't figure out why I keep getting a segmentation fault. I believe I'm allocating enough memory with malloc. Any help would be much appreciated. The code is supposed to read a day of the month and 'reminder' string from the user, and then reads these into data pointed to by the array reminders and allocated memory space with malloc.

/* Prints a one-month reminder list (dynamic string version) */

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

#define MAX_REMIND 50           /* maximum number of reminders */
#define MSG_LEN 60              /* maximum length of reminder message */

int read_line(char str[], int n);

int main(void)
{
    char *reminders[MAX_REMIND];
    char day_str[3], msg_str[MSG_LEN+1];
    int day, i, j, num_remind = 0;

    for (;;) {
        if (num_remind == MAX_REMIND) {
            printf("-- no space left --\n");
            break;
        }

        printf("Enter day and reminder: ");
        scanf("%2d", &day);
        if (day == 0) {
            break;
        } 
        sprintf(day_str, "%2d", day);
        read_line(msg_str, MSG_LEN);

        for (i = 0; i < num_remind; i++) {
            if (strcmp(day_str, reminders[i]) < 0) {
                break;
            }
        }
        for (j = num_remind; j > i; j--) {
            reminders[j] = reminders[j-1];
        }

        reminders[i] = malloc(2 + strlen(msg_str) + 10);
        if (reminders[i] = NULL) {
            printf("-- No Space Left --\n");
            break;
        }

        strcpy(reminders[i], day_str);
        strcat(reminders[i], msg_str);

        num_remind++;
    }

    printf("\nDay Reminder\n");
    for (i = 0; i < num_remind; i++) {
        printf(" %s\n", reminders[i]);
    }

    return 0;
}
int read_line(char str[], int n) {
    int ch, i = 0;

    while ((ch = getchar()) != '\n') {
        if (i < n) {
            str[i++] = ch;
        }
    }
    str[i] = '\0';
    return i;
}

Upvotes: 1

Views: 831

Answers (4)

zed_0xff
zed_0xff

Reputation: 33217

wrong:

if (reminders[i] = NULL) {

right:

if (reminders[i] == NULL) {

also, you are concatenating day_str & msg_str, so it's better to alloc:

reminders[i] = malloc(2 + strlen(msg_str) + strlen(day_str));

Upvotes: 0

Majid Azimi
Majid Azimi

Reputation: 5745

You are doing assignment in if clause. It should be like this:

if (reminders[i] == NULL) {
    printf("-- No Space Left --\n");
    break;
}

Upvotes: 0

dreamlax
dreamlax

Reputation: 95315

The dreaded assignment/comparison typo! This is a hint that your compiler warning levels are not set high enough or that you are ignoring the warnings.

// Should be '==' not '='
if (reminders[i] = NULL) {
    printf("-- No Space Left --\n");
    break;
}

Also, your read_line function is similar to fgets, which may simplify things.

Lastly, always validate your user input. Make sure scanf returns the number of items that you've asked for (usually 1 for simple one-item input). Otherwise, you're likely to venture into undefined behaviour.

Upvotes: 1

David Schwartz
David Schwartz

Reputation: 182753

    if (reminders[i] = NULL) {

You set reminders[i] to NULL before you dereference it.

Upvotes: 2

Related Questions