Print argv array

Code:

void sort_in_order(int argc, char * argv[])
{
    char * temp;
...
    strcpy(temp,argv[i]);

You don't allocate memory for "temp", thus you can't copy a string to it.
But copying the strings is the wrong approach. You need to modify the pointers.

argv[n] is a pointer to a string, i.e. it points to the memory location where the first character of the string is stored. Now it looks like your implementation stores the command line arguments consecutively like:

Code:

b  o  b \0  d  a  v  i  d \0  j  o  h  n \0  a  n  d  r  e  w \0
^           ^                 ^              ^
1           2                 3              4   (argv-indices)

Now if you swap for example argv[1] and argv[4] like you do in your sort function (using strcpy()) the result will be:

Code:

a  n  d  r  e  w \0  i  d \0  j  o  h  n \0  b  o  b \0  e  w \0
^           ^                 ^              ^
1           2                 3              4

As you can see you have just swapped the contents, but the pointers (argv[1], argv[2], ...) will still point to the same memory location as before. That's why you print out garbage and why you need to rearrange the pointers instead.

Bye, Andreas

That makes alot of sense but I think I am struggling to entirely implement that into my code. I've removed strcpy and have rearragned the pointers to the strings but it just swaps the first letters of all the names and the rest of the string says in the exact same order.

Code:

void sort_in_order(int argc, char * argv[]){
     int i, j;
     char * temp;
     
     for(i = 1; i < argc-1; i++  ){
           for(j = i+1; j < argc-1; j++){
                 if(argv[i] < argv[j]){/*I know the if statement is wrong but I'm working on the words rearragning as a whole*/
                  *temp = *argv[i];          
                  *argv[i] = *argv[j];             
                  *argv[j ]= *temp;
                 }
                 }
           
     }
}

Also, for doing rearraging the pointers do I still need to allocate memory for temp or is that only when you are copying data to temp? I have tried both ways with the same results.

First, you're a bit inconsistent in when and how you use argc or argc-1 and argv or argv+1 (or, you were in your last complete post, #14). Pick one method and stick to it. I recommend your functions don't try to adjust anything by +/- 1, that way they will work generically for any array of char *, should you want to reuse your print or sort functions. Always correct for this in main by passing in argc-1 and argv+1 to the functions. This effectively passes in a sub-array that starts at argv[1] instead of argv[0].

Code:

// in main
printf("Before sorting:\n");
print_array(argc-1, argv+1);  // adjusted in main


sort_in_order (argc-1, argv+1);  // adjusted in main
printf("\nIn order:\n");
print_array(argc-1, argv+1);  // adjusted in main
...
void print_array(int argc, char * argv[]){
    int i;
    for( i = 0; i < argc; i++){  // note, start at 0 and go up to < argc, don't try to skip the first element here, you've already adjusted in main
        printf("%s\n", argv[i]);
    }
}

All those *s before temp, argv[i] and argv[j] are the problem. Remember, temp, argv[i] and argv[j] are pointers to char, so putting a * in front of them dereferences that pointer and gives you the single char it points to, i.e. the first letter of each argument. As a side note, temp is a pointer, but it is never set to point to valid memory, so when you do the following:

Code:

*temp = argv[i];
argv[j] = *temp;

You are reading from and writing to memory you might not own or have access to. This results in undefined behavior, which may mean a crash/seg fault, garbled output, or any number of other things. Never dereference a pointer unless it points somewhere valid.

Go back and look at post #10. Your compare and swap were correct there, but I think your sorting loops were wrong. You did

Code:

for(i = 1; i < argc-1; i++  ){
    for(j = i+1; j < argc-1; j++){

You need to start i at 0, since you skip first element by passing in argv+1. Also, you need to stop at i < argc-1, since j = i+1, and this will cause a buffer overflow on the last iteration, where you will be comparing/sorting/swapping garbage. j, on the other hand, should start at i+1, and stop at j < argc. That makes j iterate over all elements after i.

Code:

for (i = 0; i < argc - 1; i++) {
    for (j = i + 1; j < argc; j++) {

That should about do it.

If you are going to be doing anything with argv other than reading from it, it's worthwhile to make a copy of it.

Thanks anduril for the help, I think the lack of consistency in my program caused a lot of unnecessary confusion. I've got the program to sort in and reverse alphabetical order so I appreciate it.

And thanks to matticus I went over all the comment from everyone. He said check my parenthesis in

Code:

if(strcmp(argv[i],argv[j]<0))

If I figured out my mistake in that sooner I might of been able to get by without asking so many extra questions