character array in bubble sort

Question

I have written the following code to get sorted strings in a 2-D character array

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

void swap(char *,char *);

void main() {
    char a[20][20];
    int Pass = 0, i = 0, j = 0, n;

    printf("\nHow many elements you want to sort ? >> ");
    scanf("%d", &n);
    printf("\n\nEnter the elements to be sorted :\n");
    for (i = 0; i < n; i++)
        scanf("%s", a[i]);
    for (Pass = 1; Pass < n; Pass++) {
        for (j = 0; j < n - Pass; j++)
            if (strcmp(a[j], a[j + 1]) < 0)
                swap(a[j], a[j + 1]);
        printf("\n\nPass = %d\n", Pass);
        for (i = 0; i < n; i++)
            printf(" %s  ", a[i]);  
    }   
}

void swap(char *a, char *b) {
    char *t;
    *t = *a;
    *a = *b;
    *b = *t;
}

But, I get output as

How many elements you want to sort ? >> 5
Enter the elements to be sorted :
1 2 3 4 5
Pass = 1
2   3   4   5   1  
Pass = 2
3   4   5   2   1  
Pass = 3
4   5   3   2   1  
Pass = 4
Segmentation fault (core dumped)

Why do I encounter segmentation fault? (the same code works properly if I use an integer array instead of a character array)

Your `swap` function is wrong. 1) `char *t; *t=*a;` : Using uninitialized variable. 2) What should be exchanged is an array rather than a pointer (or one `char`). — BLUEPIXY, Aug 28 '17 at 14:21
Save time, enable all compiler warnings: `char *t; *t=*a;` should warn that `t` is used before initialization. — chux - Reinstate Monica, Aug 28 '17 at 14:29
thanks @BLUEPIXY for pointing out, i assigned *t with memory by 't=(char*)malloc(20);' and it worked — , Aug 28 '17 at 14:29

Rohan Kumar · Answer 1 · 2017-08-29T05:45:34.547

You are messing up with your pointers in the swap method. Currently you're doing :

void swap(char *a,char *b)
{
    char *t;
    *t=*a;
    *a=*b;
    *b=*t;
}

This line *t = *a seems to be a highly likely candidate for SEGV, since t is a character pointer which is uninitialized. I ran your code via gdb and gdb also said the same thing:

Reading symbols from a.out...done.
(gdb) run
Starting program: /home/rohan/Documents/src/a.out 

How many elements you want to sort ? >> 5


Enter the elements to be sorted :
1 2 3 4 5

Program received signal SIGSEGV, Segmentation fault.
0x0000555555554a21 in swap (a=0x7fffffffdce0 "1", b=0x7fffffffdcf4 "2") at testBubble.c:26
26      *t=*a;
(gdb)

You don't need t as a pointer variable. It's just a temporary variable needed for swapping. So change your method like this, which fixed the segmentation fault in my case:

void swap(char *a,char *b)
{
    char t;
    t=*a;
    *a=*b;
    *b=t;
}

kocica · Accepted Answer · 2017-08-28T15:15:05.540

0

Entry point main should be defined as

int main()

or (with arguments)

int main(int argc, char* argv[])

but may or may not return some value.

In function swap you are accessing uninitialized pointer which causes undefined behavior. There is no need to use pointer. Use plain char.

void swap(char *a,char *b)
{
    char t;
    t=*a;
    *a=*b;
    *b=t;
}

Also to avoid buffer overflow, you should tell scanf() how many characters to scan from input buffer

scanf("%19s",a[i]);

and check if scanning succeeded.

Now you should get correct results. Here is fixed code.

edited Aug 28 '17 at 15:15

answered Aug 28 '17 at 14:21

kocica

6,412
2
14
35

C spec does not agree with your limited samples of acceptable `main()` signatures as other implementation defined, not UB, signatures are possible. – chux - Reinstate Monica Aug 28 '17 at 14:31
Thanks for info. Once somebody told me `void main` has UB and since that iam living in lie. – kocica Aug 28 '17 at 14:38
2

@FilipKočica most of the time `void main()` will work anyway. The problem here was obviously the uninitialized pointer. – Jabberwocky Aug 28 '17 at 14:57
1

@FilipKočica For some details: [int main() vs void main()](https://stackoverflow.com/questions/9356510/int-main-vs-void-main-in-c) – chux - Reinstate Monica Aug 28 '17 at 15:38

chqrlie · Answer 3 · 2017-09-03T06:36:37.570

The swap function is incorrect: you are dereferencing the uninitialized pointer t, causing undefined behavior: in your case a segmentation fault.

You should instead define t as a char:

void swap(char *a, char *b) {
    char t;
    t = *a;
    *a = *b;
    *b = t;
}

But this would only work for words with a single character.

To swap words of up to 19 bytes, use this:

void swap(char *a, char *b) {
    char t[20];
    strcpy(t, a);
    strcpy(a, b);
    strcpy(b, t);
}

And add some extra tests and protections in the main function to avoid undefined behavior:

int main(void) {
    char a[20][20];
    int Pass = 0, i = 0, j = 0, n;

    printf("\nHow many elements you want to sort ? >> ");
    if (scanf("%d", &n) != 1 || n < 0 || n > 20)
        return 1;
    printf("\n\nEnter the elements to be sorted :\n");
    for (i = 0; i < n; i++) {
        if (scanf("%19s", a[i]) != 1)
            return 1;
    }
    for (Pass = 1; Pass < n; Pass++) {
        for (j = 0; j < n - Pass; j++) {
            if (strcmp(a[j], a[j + 1]) < 0)
                swap(a[j], a[j + 1]);
        }
        printf("\nPass = %d\n", Pass);
        for (i = 0; i < n; i++)
            printf(" %s", a[i]);
        printf("\n"); 
    }
    return 0;  
}

character array in bubble sort

3 Answers3