Can someone please tell me what's wrong with my code?

Question

0.00/5 (No votes)

See more:

I'm using Dev C++ 5.5.3 and the code below compiles just fine but as soon as I enter input, my program's .exe crashes.

XML

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

main()
{
    int length;
    char *strng, *strng2;

    printf("\nPlease enter a simple phrase with no spaces: "); // ask for input
    gets(strng); // take input
    length = strlen(strng); // get length of input string minus the null terminator
    strng2 = (char*)malloc(length + 1); // allocate in the strng2 memory block the space needed for the complete string, including the null terminator
    strng--;

    while(length != 0) // fill in character by character the new array with the old array's characters in reverse order
    {
        *strng2 = *strng;
        strng--;
        strng2++;
        length--;
    }

    printf("\nThe following is the phrase you entered backward: %s\n", strng2); // displays new string

    free(strng2);
}

Posted 30-May-14 23:51pm

Aamir Yousafi

Updated 6-Jun-14 7:18am

v5

Add a Solution

Comments

Aamir Yousafi 31-May-14 5:53am

this shoudl read "#include <stdio.h>"

Thomas Daniels 31-May-14 6:01am

I edited your question so the #include line shows up correct now.

[no name] 31-May-14 6:08am

Learning how to use the debugger is a valuable skill.

Aamir Yousafi 6-Jun-14 9:24am

the debugger doesn't tell me anything because this code compiles just fine. The only issue is that it freezes as soon as I input anything

[no name] 6-Jun-14 9:36am

What exactly does "compiles just fine" have to do with the debugger?

Aamir Yousafi 6-Jun-14 9:49am

when debugging, I'm getting the error SIGSEGV segementation fault

[no name] 6-Jun-14 18:45pm

Ah so the debugger does tell you something. Probably because your pointers are corrupted.

Vedat Ozan Oner 1-Jun-14 12:45pm

does it compile? I don't expect it because of 'char strng2[k];' since 'k' is not a constant value.

Legor 2-Jun-14 4:12am

With modern c compilers this will compile. Since C99 it is possible to use variables in array deklarations (to declare arrays of variable length).

Vedat Ozan Oner 2-Jun-14 4:58am

right. it compiles. I think I'm getting old :( I don't mean since c99, but forgetting things :)

4 solutions

Solution 1

Do you want a list?

Look at what scanf does: http://www.cplusplus.com/reference/cstdio/scanf/[^]

"Reads data from stdin and stores them according to the parameter format into the locations pointed by the additional arguments."
Which means that men you call it:

C++

length = scanf("%s", &strng);

the strng parameter must contain the address of an area of memory into which characters can be put.
Your's doesn't:

C++

char *strng;

You haven't given it a value, so it contains a "random" value which may or may not be a memory location, but even if it is won't be one you can use...

For a "quick" solution, try this:

C#

char strng[80];

printf("\nPlease enter a simple phrase with no spaces: ");
length = scanf("%s", strng);

And work from there - preferably without moving the pointer to the start address...

Posted 31-May-14 0:08am

OriginalGriff

Comments

Stefan_Lang 2-Jun-14 3:05am

If you suggest a fixed string length, you might also include that info in the scanf command:

char strng[80];
scanf("%79s", strng);

P.S.: alternately you can use fgets(strng, 80, stdin);
Note that the length specifier does include the terminating 0 character, unlike scanf.

Solution 2

In addition to solution 1, some more errors:

1. scanf returns the number of items filled, which in your example is at best 1. If you want the length of the input string, check that scanf does return 1, then check the length of the string using strlen().

2. You can't define a string on the stack with a variable length:

C++

char strng2[k];

At the time of compilation, the value of k is not known, therefore the compiler will issue an error! If you want to reserve memory of just the right size, you must allocate that memory dynamically, on the heap, using malloc or calloc. Also, as pointed out above, you must use the correct length. And, in addition, you must add 1 to the length, because strings in C are terminated with a 0-Byte, and you must allocate memory for that byte as well!

3. you can't modify an array variable! You have actually managed to add several errors in just a few lines:

C++

strng = strng + i;

a) strng is defined as an array. An array is a static range within memory. The location of that range is fixed and cannot be changed. Therefore the array variable cannot be changed. The compiler should issue at least one, possibly several error messages here!
b) i was never initialized. This line would produce undefined results, if it would even compile!
c) the following code modifies too many counters at once, as well as the starting address of the string you're looking at (if it would even work). I'm not going to bother to explain why your math is wrong, since it's based on erratic code anyway...
Hint: don't use any counters at all! Just use pointers to positions within the source and destination string (and do use separate pointer variables for that purpose)

Posted 1-Jun-14 21:01pm

Stefan_Lang

Comments

Legor 2-Jun-14 4:15am

Hello Stefan, since C99 it is possible to define variable length arrays on the stack. See e.g. this documentation: http://gcc.gnu.org/onlinedocs/gcc/Variable-Length.html

Stefan_Lang 2-Jun-14 5:07am

Interesting, I wasn't aware of that - would have thought if C++11 doesn't support it, neither should C - apparently I was wrong...

CPallini 6-Jun-14 9:35am

Well, C99 is a bit 'exotic'.
By the way, my 5.

Solution 4

I finally got it working the way it needs to work! Thanks all!

C#

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

main()
{
    /* pre-run-time declarations */
    unsigned int dynlen = 256;
    int i = 0, ch = 0, j = 0;
    char *strng = (char*)malloc(dynlen);

    /* ask for input */
    printf("\nPlease enter any phrase and then press enter: ");

    /* take input of unknown string size from user and allocate and reallocate so that no memory is wasted */
    while((ch = getchar()) != '\n')
    {
        strng[i] = (char)ch;
        i++;
        if(i == dynlen)
        {
            dynlen += 256;
            strng = (char*)realloc(strng, dynlen);
        }
    }
    strng = (char*)realloc(strng, i+1);
    strng[i] = '\0';

    /* fill in character by character the new array with the old array's characters in reverse order */
    char *strng2 = (char*)malloc(i+1);
    i--;
    while(i >= 0)
    {
        strng2[j] = strng[i];
        i--;
        j++;
    }
    strng2[j] = '\0';

    /* displays new string */
    printf("\nThe following is the phrase you entered backward: %s\n", strng2);

    /* free memory */
    free(strng);
    free(strng2);
}

Posted 24-Jun-14 2:22am

Aamir Yousafi

Add a Solution

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

leon de boer · Accepted Answer · 2014-06-06T15:38:00

Solution 3

The actual error with your code if you are still interested is this line

*strng2 = *strng;

Think very carefully about what that does and look where it is.

Posted 6-Jun-14 15:38pm

leon de boer

Comments

Aamir Yousafi 7-Jun-14 10:49am

after the user inputs the character string in *strng, that pointer will be pointing to the null terminator, so after moving strng back one, it will be on the last real character of the string. ANd then with *strng2 = *strng, I set what strng2 is pointing to equal to what strng is pointing to, and then the rest of the code. I don't get what's wrong with this line, logically or in terms of syntax ...

leon de boer 8-Jun-14 0:48am

Seriously you cant see the problems of which there are three, what I was suggesting would be interesting to watch that line in the debugger :-)

1st error: strng--;

strng is pointing to the start of string ... you know it does because you used it with strlen to get it's length

so strng--; goes to I have no idea some random location in memory that is one before the allocated string memory

I assume you wanted the last character so it should have been strng = &strng[length-1];

2nd Error: strng2 gets asciiz termination exactly how?

You even noted that length doesn't include the asciiz termination character and YOU NEVER ZEROED strng2 after memory allocation so how do you think it is ever going to be asciiz terminated.

3rd error: strng is never allocated you allocate memory for strng2 but not strng therefore the program may even crash at the "gets" command because strng will be just a random value.

Look at the "gets" documentation the passed in value HAS TO BE MEMORY THAT IS ALLOCATED either malloc it like you did strng2 or assign strng as an array here is the reference on "gets"

http://www.cplusplus.com/reference/cstdio/gets/

NOTE the sample

char string [256];
gets (string);

Your options are use like the above or use the dynamic allocate alternative:

char* strng;
strng = (char*) malloc(256);
gets(strng);

Now finally a question any reason you are using Dev-C I don't think it's even supported anymore is it? You know you get microsoft VS 12 and 13 express are free if you aren't doing commercial development all you do is open a microsoft account with your e-mail it's a bit more up to date. The project settings might do your head in a little at start but that is just a matter of going thru one of the online blogs on setting up your first project.

VS 12 and 13 both correctly pick up error 3 and report it, no compiler will pick your first two errors :-)

main.cpp(588): warning C4700: uninitialized local variable 'strng' used

Aamir Yousafi 14-Jun-14 10:11am

Sorry for the late response. Your help is much appreciated.

Aamir Yousafi 24-Jun-14 5:23am

Okay. Thanks again for all advice and I have fixed issues and the program works except for one issue: after the characters are reversed, there is junk left, if i run the program a second time from the command prompt. Plz see the code below:

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

main()
{
/* pre-run-time declarations */
unsigned int dynlen = 256;
int i = 0, ch = 0, j = 0;
char *strng = (char*)malloc(dynlen);

printf("\nPlease enter a simple phrase with no spaces: "); // ask for input

/* take input of unknown string size from user and allocate and reallocate so that no memory is wasted */
while((ch = getchar()) != '\n')
{
strng[i] = (char)ch;
i++;
if(i == dynlen)
{
dynlen += 256;
strng = (char*)realloc(strng, dynlen);
}
}
strng = (char*)realloc(strng, i+1);
strng[i] = '\0';

/* fill in character by character the new array with the old array's characters in reverse order */
char *strng2 = (char*)malloc(i+1);
i--;
while(i >= 0)
{
strng2[j] = strng[i];
i--;
j++;
}

printf("\nThe following is the phrase you entered backward: %s\n", strng2); // displays new string

/* free memory */
free(strng);
free(strng2);
}

Can someone please tell me what's wrong with my code?

4 solutions

Solution 1

Solution 2

Solution 3

Solution 4

Add your solution here

Preview 0

Can someone please tell me what's wrong with my code?

4 solutions

Solution 1

Solution 2

Solution 3

Solution 4

Add your solution here

Preview 0

Existing Members

...or Join us