1

I am a not a professional so please, bear with me.

I am in the process of learning C, and for practice I made a small program that reads a couple of numbers from argv, and sorts them in ascending order.

So far it works pretty well. Except for 2 things:

  1. The program adds an extra zero to the list (I know why this is happening, but don't feel like fixing it right now)
  2. The biggest number somehow turns into a zero in the process (I don't know why this is happening and I want to fix it)

I am specifically concerned with Issue #2.

For example, when running ./a.out 6 8 4 9 3 the output comes out to 0 0 3 4 6 8 instead of 0 3 4 6 8 9 (once again I'm only concerned with the disappearance of the 9)

I am using gcc version 7.3.1 on Linux if it helps.

Why does the biggest number turn into a zero?

The code:

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

void flip(short position, short * values){
    int key = values[position];
    values[position] = values[position + 1];
    values[position + 1] = key;
}


void sort(short * values, short argc){
    for( short iterations = argc; iterations > 0; --iterations){
            for( short position = 1; position < iterations; ++position){
                    if (values[position] > values[position + 1])
                            flip(position, values);
            }
    }

    for ( short i = 0; i < argc; ++i )
            printf("%d ", values[i]);
    printf("\n");
}

int main(short argc, char **argv){
    if ( argc <= 1 ){
            fprintf( stderr, "USAGE: %s <<short int>...>\n", argv[0] );
            return 1;
    }

    short *values = malloc(sizeof(short) * (argc-1));

    for (int i = 1; i < argc; ++i ){ 
            values[i] = strtol(argv[i], NULL, 0);
    }

    sort(values, argc);

    return 0;
}

Any help would very much be appreciated.

Edit: Having had run it through GDB I figured out that the issue is in not in the main function, as x/6uh values still returns the correct values when sort starts. The issue occurs while I am inside flip. Still not sure what the problem is though...

dzerus
  • 21
  • 5
  • 7
    Take this as an opportunity to [learn how to debug your programs](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/). – Some programmer dude Jul 27 '18 at 20:40
  • 3
    However, a good start would be to think about what [`realloc`](http://en.cppreference.com/w/c/memory/realloc) ***returns*** – Some programmer dude Jul 27 '18 at 20:41
  • 2
    Since you know the size of argc, why do you need realloc? Why not just do a malloc(sizeof(short)*argc)? – Michael Miner Jul 27 '18 at 20:56
  • If only there were already a question about the "Correct Use of Realloc": https://stackoverflow.com/q/44789295/1212725 – bruceg Jul 27 '18 at 20:58
  • 1
    Although the `realloc()` usage is broken, it is probably not the key issue for small numbers of arguments. *Do* fix it, however. Do also fix your issue (1), as that shouldn't be hard and will get you thinking more about your data. – John Bollinger Jul 27 '18 at 20:59
  • See edits please – dzerus Jul 27 '18 at 20:59
  • 3
    Using `gdb` is good, but your edit makes it sound like you think you've gotten all you can out of that. I am certain that is not actually the case. Now, however, you know to focus your attention (in gdb) on the `sort` function. – John Bollinger Jul 27 '18 at 21:03
  • 3
    did you know that arrays in C start at position 0? – bruceg Jul 27 '18 at 21:03
  • I said "yet". Sorry, I meant I am trying to figure it out. – dzerus Jul 27 '18 at 21:04
  • OT: regarding: `puts("Did not specify any arguments. Exiting...");` This does not help the user to run the program properly and error messages should be output to `stderr`, not `stdout`. Normally a `usage` message is output when the user makes a mistake on the command line parameters. Suggest: `fprintf( stderr, "USAGE: %s <...>\n", argv[0] );` – user3629249 Jul 27 '18 at 21:39

1 Answers1

0

Your main problem is that you need to store the values in the array starting at values[0], and you mistakenly call your sort routine with argc which is too big. Try something like this:

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

void flip(int position, int * values){
    int key = values[position];
    values[position] = values[position + 1];
    values[position + 1] = key;
}


void sort(int * values, int numvals){
    for( int iterations = numvals - 1; iterations >= 0; --iterations){ //TODO
            for( int position = 0; position < iterations; ++position){
                    if (values[position] > values[position + 1])
                            flip(position, values);
            }
    }

    for ( int i = 0; i < numvals; ++i )
            printf("%d ", values[i]);
    printf("\n");
}

int main(int argc, char **argv){
    if ( argc <= 1 ){
            puts("Did not specify any arguments. Exiting...");
            return 1;
    }

    int *values = malloc(sizeof(int) * (argc-1));

    for (int i = 1; i < argc; ++i ){
            values[i-1] = strtol(argv[i], NULL, 0);
    }

    sort(values, argc - 1);

    return 0;
}

I changed your shorts to ints, but that likely doesn't matter except for argc.

bruceg
  • 2,433
  • 1
  • 22
  • 29
  • Ran it with your edits. When given the numbers 8 65 9 3 it returns 4259840 8 9 3. I'm not sure, maybe you removed something and I forgot to put it back though. – dzerus Jul 27 '18 at 21:29
  • I left out your `flip` function which worked fine as you had it originally. – bruceg Jul 27 '18 at 21:34
  • 1
    Yes, I noticed because it wouldn't compile without `flip` (or the headers). I must have messed up on something else though, because it works now. – dzerus Jul 27 '18 at 21:39
  • Glad you got it figured out. – bruceg Jul 27 '18 at 21:40