1

I want help regarding a function which you call from main that reverse text. However, the program works, well "more or less" but it crashes. Here is how code looks

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

void reverse(char * array, int numberOfChars) {
    int begin = 0;
    int end = 0;
    char temp;

    end = strlen(&array) - 1;
    printf("%s", &array);

    while (begin < end) {
        temp = array[begin];
        array[begin] = array[end];
        array[end] = temp;
        begin++;
        end--;
    }
}

int main() {
    reverse('supm', 4);
    return(0);
    getchar();
}

The string gets reversed to mpus, but then crashes, apparently it seems also like the arrays can only take in 4 characters, if i change it to 5 and the integer value to 5, it won't work at all. Any help would be appreciated.

Punit Vara
  • 3,744
  • 1
  • 16
  • 30
Philly
  • 13
  • 1
  • 4
  • First of all you can't have functions after `return`. Secondly, if you say it crashes, please post an error message. – Arc676 Dec 04 '15 at 14:22
  • 4
    That code should give you plenty of compiler warnings/errors. If not, check your compiler warning settings. – user694733 Dec 04 '15 at 14:23
  • 1
    Almost *everything* is wrong, I suggest you work through a few C primers before attempting this. – r3mainer Dec 04 '15 at 14:26

6 Answers6

5

strlen(&array) and printf("%s", &array); are wrong. They will pass where the pointer is instead of where the string is. use strlen(array) and printf("%s", array);.

reverse('supm', 4); is also wrong because the first argument of this call is not a string but an implementation-defined integer.

Be careful that just changing it to reverse("supm", 4); won't work because modifying string literals isn't allowed.

Also, the last getchar(); won't be executed because it is after return 0;.

Try this:

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

void reverse(char * array, int numberOfChars) {
    int begin = 0;
    int end = 0;
    char temp;

    end = strlen(array) - 1;
    printf("%s", array);

    while (begin < end) {
        temp = array[begin];
        array[begin] = array[end];
        array[end] = temp;
        begin++;
        end--;
    }
}

int main() {
    char supm[] = "supm";
    reverse(supm, 4);
    puts(supm); /* added this to see if this code is working well */
    return(0);
}

Maybe you should use the parameter numberOfChars instead of strlen(array) because you pass the parameter, but it is not used at all.

MikeCAT
  • 73,922
  • 11
  • 45
  • 70
  • I understand what you've done, but i just want to call the function from main, and what i mean by that is that i want to input "reverse('string', characters) without having to use char[supm] = "supm" if you know what i mean. – Philly Dec 04 '15 at 16:45
  • @Philly you shouldn't use `'string'` because it is implementation-defined and it may cause overflow if your size of `int` is 32 bits. Use `"string"` (string literal) instead of `'string'` (character constant), then just print and don't do the modification because you don't print the reversed string in the function, and you cannot pass the reversed string to the caller in this case unless the second parameter `characters` is a pointer or an array. – MikeCAT Dec 04 '15 at 22:36
3

A few things immediately pop out at me:

  1. strlen should take a char*, where you are passing a char**. Instead of strlen(&array), use strlen(array).
  2. Similar story with printf. Instead of printf("%s", &array), use printf("%s", array).
  3. Your string literal in main incorrectly uses single quotes, when it should be using double quotes. Your 'supm' should be "supm". You also cannot modify a string literal, so you'll need to set a variable containing your string and then pass the variable instead.
  4. You return(0) before calling getchar. That means getchar will never be called.
  5. numberOfChars is unused. Either remove that parameter or use it instead of strlen.
AlexPogue
  • 757
  • 7
  • 14
1

In your function reverse -

end = strlen(&array) - 1;
printf("%s", &array);

You pass char ** to strlen which expects a const char * and try to print a char ** with %s . Jusr pass array to both these functions. And in main -

reverse('supm', 4);

If you even do "spum" which would be string literal , thus constant (modifying it would cause problem).

Do this instead -

char s[]="spum"; 
reverse(s, 4);

Compiler must have issued warnings for these statements and if not then enable compiler warnings.

ameyCU
  • 16,489
  • 2
  • 26
  • 41
  • What if, in the function at the start you say `char s[] = array`, and then worked off of `s`? would this allow you to pass a string literal? – Adjit Dec 04 '15 at 14:42
  • @Adjit No. it wouldn't. It will emit [compile error](http://melpon.org/wandbox/permlink/QYTNDrdzqE78HtgC). – MikeCAT Dec 04 '15 at 14:47
  • @Adjit I guess than you would need to use `strcpy` for that to work . You cannot assign values to arrays just you do to variables . – ameyCU Dec 04 '15 at 14:48
  • @Adjit Copying string literal to such array will work . **Note** - Not passing `"spum"` directly . – ameyCU Dec 04 '15 at 14:54
  • How would you use `strcpy`? Just pass the return value to the function? `reverse(strcpy("spum"), 4)` – Adjit Dec 04 '15 at 15:03
  • @Adjit `strcpy` inside function . – ameyCU Dec 04 '15 at 15:22
1
  1. arrayis already a pointer-- &arraygives you a pointer to the pointer. so change the strlen call to this:

    strlen(array);
    
  2. you need to define the string before you pass to the function-- otherwise the string only exists inside your function, you won't be able to access it outside.

    char mystr[] = "supm";
    reverse(mystr, 4);
    printf("%s\n", mystr);
    return 0;
    
Erik Nyquist
  • 1,267
  • 2
  • 12
  • 26
  • 1
    Do not remove the -1 because `array[strlen(array)]` is the null character, which you won't want. – MikeCAT Dec 04 '15 at 14:32
  • @ameyCU If `char array[]="spum";` `array[0]='S'`, `array[1]='p'`, `array[2]='u'`, `array[3]='m'`, `array[4]='\0'`. so the trouble will happen. – MikeCAT Dec 04 '15 at 14:36
  • @MikeCAT Yes , realized that it was inside `[]` , therefore , removed my comment earlier. – ameyCU Dec 04 '15 at 14:37
  • Ah yes you're right-- I was mistakenly thinking OP wanted the string length. It seems he actually wants index of last readable char, so yes this would cause an issue! Have removed it. – Erik Nyquist Dec 04 '15 at 14:38
1

Try this code I have fixed all your code problems.

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

void reverse(char * array, int numberOfChars) {
    int begin = 0;
    int end = 0;
    char temp;

    end = strlen(array) - 1;
    printf("%s\n", array);

    while (begin < end) {
        temp = array[begin];
        array[begin] = array[end];
        array[end] = temp;
        begin++;
        end--;
    }
}

int main() {
    char str[] ="sump";
    reverse(str, 4);
    puts(str);    
    return(0);
    
}

What you can learn from problems of this code ?

void reverse(char * array, int numberOfChars) /* definition */

reverse('supm', 4); /*calling function */

Compare this both two in definition first argument is to pass character array char *array But in calling you are passing'sump' .Now onwards please remember for more than character you can use "" so here you should use "sump"

end = strlen(&array) - 1;

Refer strlen

Its argument is constant character pointer . But here you are passing &array .I guess you think &array is address of array but that is wrong for array array represent address and array[i] represent value

getchar()

This function is for reading input not for printing strings. refer getchar

For printing reversed output you should use puts at last not getchar

Community
  • 1
  • 1
Punit Vara
  • 3,744
  • 1
  • 16
  • 30
0

In your main function, the line reverse('supm', 4); has a couple of things wrong with it. First of all, it is a character constant, not a string, so you will end up passing an invalid pointer value to your reverse function. You probably meant to use "supm" which is a string and does match the char * expected by your reverse function. However, "supm" is a string literal constant, so although you can point to it, you are not allowed to modify it. So you should do something like this:

    char rev[] = "supm";
    reverse(rev, 4);

Here, rev is an array of 5 char, initialized as {'s', 'u', 'p', 'm', '\0'}.

You are not currently using the numberOfChars parameter of reverse, so you could remove it.

Ian Abbott
  • 15,083
  • 19
  • 33