strings of characters in C

Question

I am trying to solve a C problem where I have to sort n strings of characters using pointers.

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

void sort (char *s, int n)
{
    int i,j; char aux[]="";
    for (i=1;i<=n-1;i++)
    {
        for (j=i+1;j<=n;j++) //if s[i]> s[j] switch
        {
            if (strcmp(*(s+i),*(s+j))==1)
            {
                strcpy(aux,*(s+i);
                strcpy(*(s+i),*(s+j));
                strcpy(*(s+j),*(s+i));
            }
        } 
    }

}

void show(char *s, int n)
{
    int i;
    for (i=1;i<=n;i++)
    {
        printf("%s",*(s+i));
    }
}
int main()
{
    int i,n; char *s;
    printf("give the number of strings:\n");
    scanf("%d",&n);
    s=(char*)calloc(n,sizeof(char));
    for (i=1;i<=n;i++)
    {
        printf("s[%d]= ",i);
        scanf("%s",s+i);
    }
    sort(s,n);
    show(s,n);


    return 0;
}

The warnings that I get are when I use strcmp to compare and when I use strcpy to switch *(s+i) and *(s+j) values.

"passing argument 2 of strcmp makes pointer from integer without a cast"

result of strcmp is rarely equal to 1. You probably want to test if it is greater than zero. Also it seems you are confusing string and char type in the whole program. — Marian
– Marian, Commented Jan 8, 2014 at 16:04
There are many problems with this code, but the most fundamental problem is that you do not have an array of strings to sort - you just have one string of n chars. — Paul R
– Paul R, Commented Jan 8, 2014 at 16:07
s=(char*)calloc(n,sizeof(char)); -> s=(char*)calloc(n+1,sizeof(char)); +1 for terminator('\0'). — BLUEPIXY
– BLUEPIXY, Commented Jan 8, 2014 at 16:15

Sri Harsha Chilakapati · Accepted Answer · 2014-01-08 16:28:17Z

3

The warnings that I get are when I use strcmp to compare and when I use strcpy to switch
*(s+i) and *(s+j) values.
"passing argument 2 of strcmp makes pointer from integer without a cast"

You are wrong argument to strcmp and strcpy. Signature of strcmp and strcpy are

int strcmp(char *string1, char *string2); 
char *strcpy(char *dest, const char *src)

But you are passing it the arguments of char type. Removefrom(s+i)and*(s+j)`. It should be

if (strcmp((s+i), (s+j))==1)
{
      strcpy(aux, (s+i));
      strcpy((s+i), (s+j));
      strcpy((s+j), (s+i));
}

Another problem is you have not allocated memory for the pointer s. For characters, you can declare it as

 s = malloc ( (n + 1)*sizeof(char) );

or simply

 s = malloc ( n + 1 ); // +1 is for string terminator '\0'

edited Jan 8, 2014 at 16:28

Sri Harsha Chilakapati

12k6 gold badges54 silver badges91 bronze badges

answered Jan 8, 2014 at 16:03

haccks

107k28 gold badges181 silver badges274 bronze badges

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

3 Comments

user3078259 Over a year ago

can anyone show me how do I allocate memory for n strings? it seems like this is the biggest problem.

user3078259 Over a year ago

haccks, isn`t this how you allocate memory for n characters? because I need to allocate memory for n strings of characters.

haccks Over a year ago

For n strings, you need to declare an array of pointers and then allocate memory by using malloc for each element of the array.

unwind · Accepted Answer · 2014-01-08 16:15:52Z

1

You seem to have mis-understood the return value of strcmp(); it's an integer that will be 0 (zero) if and only if the two string arguments are equal. You're testing for 1, which it will only be rarely; it has no specific meaning.

Consider just using qsort().

answered Jan 8, 2014 at 16:15

unwind

402k64 gold badges492 silver badges620 bronze badges

Comments

Nerf Herder · Accepted Answer · 2014-01-08 16:29:17Z

1

You're allocating space for n strings of 1 byte each. The easiest approach is to assume each string will be less than some set length, like 40 bytes. Then you would allocate the memory like this:

s=(char*)calloc(n*(40),sizeof(char));

Then your scanf needs to be modified:

scanf("%s",s+(i*40));

Now, string 1 will be at *s, string 2 will be at *(s+40), etc. Keep in mind that a string ends with a null character (0x00), so the string can only contain 39 chars. Any unused data will also be 0x00. Do the same s+(i*40) for the sorting algorithm, compare to >0, not ==1, and strcmp, strcpy expect pointers. Then you should be good.

answered Jan 8, 2014 at 16:29

Nerf Herder

4143 silver badges11 bronze badges

Comments

BenMorel · Accepted Answer · 2014-08-03 06:25:39Z

In your code there are a lots of mistakes: in variable declarations, memory allocation and pointer usage.

First remember that you should always reserve enough space to store your string values, and be aware to don't try to store a string with a length greater then the reserved space.

Also you should be very careful managing pointers (s+1 doesn't point to the first string but to the second one, s+n points out of the allocated memory)

So, as mentioned in other answers you should decide the maximum size of your strings an then allocate for them the right amount of memory. Then I suggest you to use a pointer to strings, i.e. a char** to access your strings and manage the sort, in this way the code is more readable and the sort is faster (you don't need to copy strings, but only to switch pointers)

So your main function should be:

int main()
{
  int i, n;

  // Declare the pointer to the memory where allocate the space for the strings
  char* a;  // points to a char ( == is a string)

  // Declare the pointer to the memory where store the pointer to the strings
  char** s; // points to a char* ( == is a pointer to a string)

  printf("give the number of strings:\n");
  scanf("%d", &n);

  // Allocate the memory for n strings of maximum 40 chars + one more for the null terminator
  a=(char*)calloc(n*41, sizeof(char));

  // Allocate memory for n pointers to a string
  s=(char**)calloc(n, sizeof(char*));

  // Notice the 0-based loop
  for (i=0; i<n; i++)
  {
    s[i] = a + i*41; // set the i-th element of s to point the i*41-th char in a
    printf("s[%d]= ", i);
    scanf("%40s", s[i]); // read at least 40 characters in s[i], 
                        // otherwise it will overflow the allocated size 
                        // and could generate errors or dangerous side effects
  }

  sort(s,n);
  show(s,n);

  // Remember always to free the allocated memory
  free(s);
  free(a);

  return 0;
}

the sort function sorts an array of pointers, without copying strings:

void sort (char** s, int n)
{
  int i, j; 
  char* aux;

  // Notice the 0-based loop
  for (i=0; i<n; i++)
  {
    for (j=i+1; j<n; j++) //if s[i]> s[j] switch
    {
      if (strcmp(s[i], s[j])>0)
      {
        // You don't need to copy strings because you simply copy pointers
        aux = s[i];
        s[i] = s[j];
        s[j] = aux;
      }
    } 
  }
}

finally the correct show function:

void show(char** s, int n)
{
  int i;

  // Notice the 0-based loop
  for (i=0; i<n; i++)
  {
    printf("%s", s[i]);
  }
}

I can't test the code but it should works

Added other options

1) If you have troubles with dynamic memory allocation you can use instead static allocation, your main function then will be:

int main()
{
  int i, n;

  // Declare an array of fixed lenght strings
  char s[100][41]; // You manage max 100 strings

  printf("give the number of strings:\n");
  scanf("%d", &n);

  // Ensure that n is less or equal maximum
  if (n>100)
  {
    printf("you'll be asked for a maximum of 100 strings\n");
    n=100;
  }

  // Notice the 0-based loop
  for (i=0; i<n; i++)
  {
    printf("s[%d]= ", i);
    scanf("%40s", s[i]); // read at least 40 characters in s[i], 
                         // otherwise it will overflow the allocated size 
                         // and could generate errors or dangerous side effects
  }

  sort(s,n);
  show(s,n);

  // You don't need to free any memory

  return 0;
}

all other functions remain unchanged.

2) Whereas, if you want the maximum freedom in the memory you need you can choose a two allocate memory separately for each string only for the needed size, in this case your main funcion will be:

int main()
{
  int i, n;

  char buffer[1000]; // You need a buffer to read input, before known its actual size

  // Declare the pointer to the memory where store the pointer to the strings
  char** s; // points to a char* ( == is a pointer to a string)

  printf("give the number of strings:\n");
  scanf("%d", &n);

  // Allocate memory for n pointers to a string
  s=(char**)calloc(n, sizeof(char*));

  // Notice the 0-based loop
  for (i=0; i<n; i++)
  {
    printf("s[%d]= ", i);
    scanf("%999s", buffer); // read at least 999 characters in buffer, 
                            // otherwise it will overflow the allocated size 
                            // and could generate errors or dangerous side effects

    // Allocate only the memory you need to store the actual size of the string
    s[i] = (char*)calloc(strlen(buffer)+1, sizeof(char)); // Remember always 1 more char for the null terminator

    // Copy the buffer into the newly allocated string
    strcpy(s[i], buffer);
  }

  sort(s,n);
  show(s,n);

  // Remember always to free the allocated memory
  // Now you have first to free the memory allocated for each string
  for (i=0; i<n; i++)
  {
    free(s[i]);
  }

  // Then you can free the memory allocated for the array of strings
  free(s);

  return 0;
}

all other functions remains unchanged too.

Collectives™ on Stack Overflow

strings of characters in C

4 Answers 4

3 Comments

Comments

Comments

Added other options

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

3 Comments

Comments

Comments

Added other options

Comments

Your Answer

Sign up or log in

Post as a guest

Related