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.

3
  • First of all you can't have functions after return. Secondly, if you say it crashes, please post an error message. Commented Dec 4, 2015 at 14:22
  • 4
    That code should give you plenty of compiler warnings/errors. If not, check your compiler warning settings. Commented Dec 4, 2015 at 14:23
  • 1
    Almost everything is wrong, I suggest you work through a few C primers before attempting this. Commented Dec 4, 2015 at 14:26

6 Answers 6

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.

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

2 Comments

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 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.
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.

Comments

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.

6 Comments

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 No. it wouldn't. It will emit compile error.
@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 .
@Adjit Copying string literal to such array will work . Note - Not passing "spum" directly .
How would you use strcpy? Just pass the return value to the function? reverse(strcpy("spum"), 4)
|
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;
    

4 Comments

Do not remove the -1 because array[strlen(array)] is the null character, which you won't want.
@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 Yes , realized that it was inside [] , therefore , removed my comment earlier.
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.
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

4 Comments

Did you mean to post some modified code? Because you actually posted the original code!
As I see questioner is beginner in c I have just gave him similar working code and show him where he did mistakes so he can understand c and do not repeat same mistakes next time.
You've changed it now, but your changed code still has the incorrect strlen(&array) from the original code.
Sorry @IanAbbott I have mistakenly paste wrong code instead of my compiled one. Thanks for suggestion .I have updated the answer .
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.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.