2

I've tried to take a certain string, have the string set for a certain size, 50 chars, then after all of the strings gets inputted, it will sort them, then realloc the sizes from 50 chars to the length of the string that the user wrote, if at first I gave it 50 bytes, and somebody entered "hi" it will change to the amount of bytes needed.

#include <stdio.h>

#define MAX_CHARS 50

int main(void)
{
    int i = 0, j = 0;
    char* temp = 0;
    char** names = 0;
    int amount = 0;

    // Getting number of friends from user
    printf("Enter number of friends: ");
    scanf("%d", &amount);
    getchar();

    // Allocating space for the names.
    temp = (char*)malloc(MAX_CHARS * sizeof(char));
    names = (char*)malloc(amount * sizeof(char));

    for (i = 0; i < amount; i++)
    {
        names[i] = (char*)malloc((MAX_CHARS + 1) * sizeof(char));
    }

    // Getting the names from the user
    for (i = 0; i < amount; i++)
    {
        printf("Enter name of friend %d: ", i + 1);
        fgets(names[i], MAX_CHARS - 1, stdin);
    }

    for (i = 0; i < amount; i++)
    {
        for (j = i + 1; j < amount; j++)
        {
            if (strcmp(names[j], names[i]) < 0)
            {
                strcpy(temp, names[j]);
                strcpy(names[j], names[i]);
                strcpy(names[i], temp);
            }
        }
        // Reallocating the 50 bytes space to only the space needed.
        printf("%d", strlen(names[i]));
        (*names)[i] = (char*)realloc((*names)[i], strlen(names[i]) * sizeof(char));
    }

    for (i = 0; i < amount; i++)
    {
        printf("%s", names[i]);
    }

    free(names);
    getchar();
    return 0;
}
Improve this question
2
  • 3
    What is your question? Commented May 3, 2020 at 20:03
  • @Meir Biton Starting from this names = (char*)malloc(amount * sizeof(char)); the code is invalid. Commented May 3, 2020 at 20:07

3 Answers 3

Reset to default
2

names is an array of pointer to char, so in

names = (char*)malloc(amount * sizeof(char));

you do not allocate enough, and later the behavior will be undefined when you will assign it out of it

do (the cast is useless)

names = malloc(amount * sizeof(char*));

Doing

(*names)[i] = (char*)realloc((*names)[i], strlen(names[i]) * sizeof(char));

is invalid because (*names)[i] is a char, do not forget also the place for the null character ending the string, so you want :

names[i] = realloc(names[i], strlen(names[i]) + 1);

note that by definition sizeof(char) is 1

Without checking the result of malloc and realloc you suppose/hope there is enough memory, but this may be false and in that case these functions returns NULL, it is more safe to check that case. That means for the realloc to first save in an auxiliary char* to not lost the current value of names[i] you can continue to use if realloc returns NULL

To do

scanf("%d", &amount);

is dangerous, when the input is invalid you do not know that and amount is not initialized with an undefined behavior when you use it, do for instance

if (scanf("%d", &amount) != 1)
{
   puts("invalid value");
   return -1;
}

Considering how you use names[i] when you do

names[i] = (char*)malloc((MAX_CHARS + 1) * sizeof(char));

you allocate 1 byte too much, you can do

 names[i] = malloc(MAX_CHARS);

Warning doing :

fgets(names[i], MAX_CHARS - 1, stdin);

the probable newline ending your input is saved in names[i], probably you need to remove it. In that case you have to adapt when you print the names to introduce a separator between the names, a space or a newline.

An other way to read but without getting the newline is :

 scanf(" 49%[^\n]", names[i]);

The 49 allows to limit the number of characters written in the array (I removed 1 to let the space for the null char), and the spaces before allows to bypass the spaces at the beginning of the input (here spaces means ' ' but also tab, newline etc). Using that way the name can contains spaces, which is not the case with the format "%49s".

Anyway whatever you use you need to check the input was done, else you do not set the array and when you will use it later the behavior will be undefined.

When you sort your array you do :

strcpy(temp, names[j]);
strcpy(names[j], names[i]);
strcpy(names[i], temp);

but you do not need to copy in deep, just exchange the pointers it it :

char * aux = names[j];

names[j] = names[i];
names[i] = aux;

At the end you want to free the resources, but you do only free(names); so you do not free the other arrays

Improve this answer
Sign up to request clarification or add additional context in comments.

5 Comments

Technically, it is possible for realloc() to return NULL even when shrinking the block. names[i] = realloc(names[i], strlen(names[i]) + 1); would cause a memory leak in this case. Storing the return value to a pointer and testing for NULL is the preferred pattern for most uses of realloc().
when I try to realloc, it triggers a breakpoint. Did exactly what you said
@MeirBiton you need also to fix the first erroned malloc, not only the realloc
Aside: "names is an array of pointer to char" --> char **names is a pointer to a pointer to char. names is not an array.
"An other way to read but without getting the newline is : scanf(" 49%[^\n]", names[i]);" --> this creates troubles when the line of input is only "\n".
1

How to use realloc to shorten a string array size

Wrong allocation

//                                v----------v s/b the size of a pointer 
// names = (char*)malloc(amount * sizeof(char));
names = malloc(sizeof *names * amount);
//             ^-----------^ Much easier to code right

off-by-1 (or 2) errors

// fgets(names[i], MAX_CHARS - 1, stdin);
fgets(names[i], MAX_CHARS + 1, stdin);

// realloc((*names)[i], strlen(names[i]) * sizeof(char));
realloc((*names)[i], (strlen(names[i]) + 1)* sizeof(char));

leaving \n in input

fgets(names[i], MAX_CHARS - 1, stdin);
// add
names[i][strcspn(names[i], "\n")] = '\0'; // to lop off potential \n

Potential UB with mis-matched printf specifier

// printf("%d", strlen(names[i]));
printf("%zu", strlen(names[i]));

Failure to free allocations

// add before `free(names);`
for (i=0; i<amount; i++) free(names[i]);

Inefficient sort

Code swaps names, when only the pointers to the names need swapping. Also consider qsort()

Suggested middle code that leaves out the sort details. Recommend to sort after all names entered.

// Allocating space for the names.

// No need to allocate, a simple array will do.
// Let us double it size to help detect and consume long-ish names
char temp[MAX_CHARS * 2];

names = malloc(sizeof *names * amount);
if (names == NULL) Handle_OutOfMemory();

// Getting the names from the user
for (i = 0; i < amount; i++) {
    printf("Enter name of friend %d: ", i + 1);
    if (fgets(temp, sizeof temp, stdin)) {
      Handle_Unexpected_Eary_EOF();
    }
    temp[strcspn(temp, "\n")] = '\0'; // lop off potential \n 
    size_t len = strlen(temp);
    if (len > MAX_CHARS) Handle_LongLine();

    names[i] = malloc(len + 1);  // only allocated what is needed
    if (names[i] == NULL) Handle_OutOfMemory();
    strcpy(name[i], temp);
}

for (i = 0; i < amount; i++) {
    printf("<%s>\n", names[i]);
}

// Sort by your own code or take time to learn `qsort()`
qsort(names, amount, sizeof names[0], TBD_compare_function);

for (i = 0; i < amount; i++) {
    printf("<%s>\n", names[i]);
    free(names[i]);
}
free(names);
Improve this answer

Comments

0 
#include <stdio.h>
#include <stdlib.h>
#include <errno.h>

#define SIZE 50
#define NEWSIZE 25

int main(void)
{
        char *str = malloc(SIZE);

        /* now there are 25 bytes
         * allocated for str, unless
         * an error occurs
         */
        void *tmp = realloc(str, NEWSIZE);
        if (!tmp) {
                perror("ERROR");
                exit(EXIT_FAILURE);
        }
        str = tmp;
        exit(EXIT_SUCCESS);
}
Improve this answer

3 Comments

Suggest void *tmp = realloc (str, 25); if (!void) { /* handle error */ }; str = tmp; This will avoid the loss of the original pointer when realloc fails returning NULL.
I had avoided such for the sake of simplicity, but alright.
Sure, no ding, and in the case of exiting regardless -- it doesn't really make a difference. But always good to show proper practice in the general case to cover the OP taking your solution and popping it into a function.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.