patterncMinor
Learning C, basic string reversal function
Viewed 0 times
functionlearningreversalstringbasic
Problem
I am starting to write some functions while studying how to program in C. In this small function I take a string as an argument and print it out in reverse order.
I was hoping to get some tips on best practice or corrections on any errors.
#include
#include
int main(int argc, char *argv[]) {
int aLen = argc;
char *sWord = argv[aLen - 1];
char *reversed;
int sLen = strlen(sWord);
int i = 0;
reversed = reversed + sLen; // offset pointer to the length of string
while (i <= sLen) {
if (sWord[i] != '\0') { // check if character is null-terminator
*reversed = sWord[i]; // if not, let reversed correct address
} // equal letter.
i++;
reversed--;
}
printf("%s reversed is:%s\n", sWord, reversed);
return 0;
}I was hoping to get some tips on best practice or corrections on any errors.
Solution
Your code has a number of issues, the main one being that it doesn't work.
If you want to reverse a string in place then I suggest you write a function just
for that, separate from
and will, for example:
To swap the characters pointed to by
or if you have a pointer and a length:
So your reversal function needs to contain one of these sequences in a loop
and the loop must arrange
example, here is a reverse function:
Then in
string:
Of course it is best to check that
Notes:
In your code,
not initilaised but is then used in
In the line
EDIT
When you declare a variable local to a function, the compiler sets aside a
space on the stack. It does not write anything there unless you declare
the initial value for the variable. So in your code,
whatever happens to be at that location on the stack and, as
pointer, it therefore points to some unknown (unplanned) location in memory.
If you are unlucky that location will be writeable, your accesses 'work'
and you won't see the problem (or not immediately - in a larger program you
will probably encounter strange behaviour later on while the code is executing
because your accesses have overwritten something important); if you are
lucky and the location is unwritable you will see the problem the first
time you run the program.
The moral is always to initialise variables, just to be safe. Zero (NULL) is
a good value for pointers. With C99 you are not restricted to defining them at
the start of a function, so it is often best to define a variable only at the
point where you need it and have a useful value with which to initialise it.
If you want to reverse a string in place then I suggest you write a function just
for that, separate from
main. The function will need a temporary variableand will, for example:
swap first and last characters
swap 2nd and 2nd-from-last characters
swap 3rd and 3rd-from-last characters
etcTo swap the characters pointed to by
a and b, you would dochar temp = *a;
*a = *b;
*b = *temp;or if you have a pointer and a length:
char temp = a[0];
a[0] = b[len];
b[len] = temp;So your reversal function needs to contain one of these sequences in a loop
and the loop must arrange
a and b to point to the correct places. Forexample, here is a reverse function:
static char* reverse(char *str)
{
char *s = str;
char *end = s + strlen(s) - 1;
for (; s < end; ++s, --end) {
char ch = *end;
*end = *s;
*s = ch;
}
return str;
}Then in
main all you do is call the reverse function with the desiredstring:
printf("'%s'\n", reverse(argv[1]));Of course it is best to check that
argv[1] exists first (remember that argv[0] is the program name and argc is always greater than or equal to 1):if (argc == 2) {
printf("'%s' reversed is ", argv[1]);
printf("'%s'\n", reverse(argv[1]));
}Notes:
In your code,
aLen and sWord are unnecessary. And reversed isnot initilaised but is then used in
reversed = reversed + sLen; // See the EDIT belowIn the line
int sLen = strlen(sWord);strlen returns the length as a size_t, not intEDIT
When you declare a variable local to a function, the compiler sets aside a
space on the stack. It does not write anything there unless you declare
the initial value for the variable. So in your code,
reserved containswhatever happens to be at that location on the stack and, as
reserved is apointer, it therefore points to some unknown (unplanned) location in memory.
If you are unlucky that location will be writeable, your accesses 'work'
and you won't see the problem (or not immediately - in a larger program you
will probably encounter strange behaviour later on while the code is executing
because your accesses have overwritten something important); if you are
lucky and the location is unwritable you will see the problem the first
time you run the program.
The moral is always to initialise variables, just to be safe. Zero (NULL) is
a good value for pointers. With C99 you are not restricted to defining them at
the start of a function, so it is often best to define a variable only at the
point where you need it and have a useful value with which to initialise it.
Code Snippets
swap first and last characters
swap 2nd and 2nd-from-last characters
swap 3rd and 3rd-from-last characters
etcchar temp = *a;
*a = *b;
*b = *temp;char temp = a[0];
a[0] = b[len];
b[len] = temp;static char* reverse(char *str)
{
char *s = str;
char *end = s + strlen(s) - 1;
for (; s < end; ++s, --end) {
char ch = *end;
*end = *s;
*s = ch;
}
return str;
}printf("'%s'\n", reverse(argv[1]));Context
StackExchange Code Review Q#27003, answer score: 5
Revisions (0)
No revisions yet.