Click here to Skip to main content
15,563,054 members
Please Sign up or sign in to vote.
1.00/5 (1 vote)
See more:
Problem 1:
I'am not sure if i have used array of character pointers correctly or not.
Problem 2:
The use of scanf()? Can we input a string in string[i](which is actually a element on array of character pointer)?(You can ignore the use of fflush(stdlin) in my code it was just used to remove the enter key before taking any input from the buffer)(just ignore it).
Problem 3:
It is actually taking first two input and then terminates the program 😒;
OUTPUT
Parth
vijay

after this it terminates;

What I have tried:

C


#include <stdio.h>
#include <stdlib.h>
#include <string.h>
int main()
{
    int i, j;
    char *item;
    char *string[10];
    for (i = 0; i < 10; i++)
    {
        fflush(stdin);
        scanf("%s",string[i]);
    }
    for (i = 0; i < 10; i++)
    {
        for (j = i + 1; j < 10; j++)
        {
            if (strcmp(string[i], string[j]) == 1)
            {
                strcpy(item, string[i]);
                strcpy(string[i], string[j]);
                strcpy(string[j], item);
            }
        }
    }
    for (i = 0; i < 10; i++)
    {
        printf("%s\n", string[i]);
    }
    return 0;
}
Posted
Updated 2-Oct-22 3:51am
v2

Numbers like 10 and 15 are better with names, it makes it easier to read. That memory must be allocated for all strings has already been written. You could get the memory with only one malloc at a time and then allocate only pointers. It would be more efficient for small amounts of memory and you only have to check once if you got the memory. Memory requested with malloc should be freed with free() as soon as it is no longer needed.
C
#define STCOUNT 10
#define STLEN 15

int i, j;
char *baseptr, *item, *string[STCOUNT];

baseptr = (char*)malloc((STLEN+1)*STCOUNT); // space for 15 characters plus terminating null
if (baseptr == NULL) { exit(11); }

item = baseptr;
for (i = 0; i < STCOUNT; i++) {
   string[i] = item;
   item += (STLEN + 1);
   // scanf("%d: %s", i, string[i]);
   scanf("%15s", string[i]);   // limit scanf to 15 characters
   // scanf_s( "%15s", string[i], STLEN);   // better use secure version
}
// ...
if (baseptr)
	free(baseptr);

Using the scanf function with the %s option without a length limit is very careless. Besides alternatives like fgets() you could at least specify the maximum length with %s. Recently there are also the secure versions with _s.

Instead of copying all letters of the strings several times, it would be much more efficient to swap only the pointers:
C
if (strcmp(string[i], string[j]) == 1) {
	// strcpy(item, string[i]);
	item = string[i];
	// strcpy(string[i], string[j]);
	string[i] = string[j];
	// strcpy(string[j], item);
	string[j] = item;
}


If you need only small amounts of memory, as in this case, you could also save the effort with malloc() completely and define the field like this:
C
char string[STCOUNT][STLEN + 1];
 
Share this answer
 
v3
Comments
Parth Vijay 2-Oct-22 23:31pm    
You have used basepointer so that we don't have to free the item pointer again and again?
and one more thing Is it valid to free the basepointer before making item pointer free?
Although, thanks for the solution.
merano99 3-Oct-22 3:34am    
The basepointer contains the pointer to the only area requested with malloc(). There is only one malloc(), so a free() is also sufficient. The element pointer "item" contains a copy of basepointer and is then only offset. Therefore you must not free the element pointer separately as well.
Parth Vijay 3-Oct-22 3:37am    
ok got it...:)
You cannot read (or copy) text data into arrays that do not exist. Your allocation of an array of pointers is correct, but you then need to initialise each pointer, so that it actually points to some memory space, something like:
C++
for (i = 0; i < 10; i++)
{
    fflush(stdin);
    string[i] = (char*)malloc(16); // allocate space for 15 characters plus terminating null
    scanf("%s",string[i]);
}

You also need to do the same for item otherwise that will also crash your program.
 
Share this answer
 
Comments
merano99 2-Oct-22 8:59am    
Swapping the strings by copying the letters is not a good idea. It is better to swap only the pointers, then you don't need additional memory for item. See my proposed solution.
Richard MacCutchan 2-Oct-22 9:11am    
Yes, I am well aware of that, you should be telling OP, rather than me.
merano99 2-Oct-22 18:22pm    
My answer was the reaction to your "You also need to do the same for item". Here it would be just not good to do the same, but to use pointers directly instead of malloc.
Richard MacCutchan 3-Oct-22 4:20am    
My answer was based on the original question and getting that problem fixed. Switching to the better way of doing things (as you rightly suggest) would maybe just confuse the OP, IMHO.
Parth Vijay 3-Oct-22 6:56am    
Yes sir, The first need was to know the mistake in my own code then moving on to some other ideas..

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900