simple function problem

[David Reghay]

I've tried altering this program a few times... it compiles fine but it crashes when I run it immediately after input. Anyone know what I've done wrong? By the way, I know I could very easily accomplish what this program is designed to do without the convert() function, but I wrote this as an exercise designed to introduce me to using return values of functions.

#include <stdio.h>

float convert(void);


float dollars;


int main(void)
{
    float pounds;


    printf("Enter a dollar amount: $");
    scanf("%f", dollars);


    pounds = convert();


    printf("That's equal to %f pounds!", pounds);


    return 0;
}


float convert(void)
{
    return dollars*2;
}

[anduril462]

Your code should generate a warning like the following:

$ make foo
gcc -Wall -Wunreachable-code -ggdb3  -lm -lpthread  foo.c   -o foo
foo.c: In function ‘main’:
foo.c:15: warning: format ‘%f’ expects type ‘float *’, but argument 2 has type ‘double’

If it doesn't, turn the warning level up.

scanf("%f", &dollars);

You need that & in there, so scanf has the address of where to store the information it read from the user. As you have it, without the &, it takes the value from pounds (which is garbage, since you haven't initialized it), and tries to use that as the address, and that will very likely produce an invalid memory address. When scanf tries to write to that bogus address, your program crashes.

[laserlight]

This:

scanf("%f", dollars);

should have been:

scanf("%f", &dollars);

If you had compiled at a higher warning level, it is likely that your compiler would have warned you about this.

By the way, avoid global variables: you could have passed the dollar value as an argument to the convert function instead.

[David Reghay]

Thanks for the fast reply, guys, I can't believe I forgot the ampersand for input through scanf()

[David Reghay]

Here's the tidied up code. I tried to use all your suggestions. I don't know how to turn up the warning level on my compiler, if anyone can help with that. I'm using visual studio express. Please let me know if this is still a sloppy implementation.

#include <stdio.h>

float convert(void);


int main(void)
{
	float pounds;


	printf("Enter a dollar amount: $");
	pounds = convert();


	printf("That's equal to %.2f pounds!", pounds);


	return 0;
}


float convert(void)
{
	float dollars;
	scanf("%f", &dollars);


	return dollars*2;
}

[William Putnam]

I don't know how to turn up the warning level on my compiler, if anyone can help with that. I'm using visual studio express. Please let me know if this is still a sloppy implementation.

I have VS 2010 as my compiler, but I'm sure that the preference settings can be the same. Go to Project > *.exe name* Properties, then go under the Configuration Properties drop down, then under the C/C++ drop down, then go under General. Now as to what settings you use, it all depends on what your personal preferences are. I personally use the Level 4 warning level, but don't treat warnings as errors. I hope this helps.

Also, if you get the warning about not using printf_s(), you can disable that. Under the C/C++ drop down, go under Advanced, then next to "Disable Specific Warnings" type 4996, then click Apply.

[anduril462]

Here's the tidied up code. I tried to use all your suggestions. I don't know how to turn up the warning level on my compiler, if anyone can help with that. I'm using visual studio express. Please let me know if this is still a sloppy implementation.

Not sloppy, but I think you misunderstood laserlight's suggestion. It was to make dollars a parameter to the function. A function should do one thing and do it well. If it's job is to convert, it should only convert, not take user input also. Another benefit of doing this is that your function can better be reused. Imagine you want to read dollar values in from a file in some sort of automated script (e.g. runs overnight, with no user). You don't want to write a whole separate convert function to do that, you want to reuse the convert function you already have. You can't do that if the function requires user input though. Also, convert is a bit generic for a name, it could convert anything to anything. Try something like convert_dollars_to_pounds. Here's the way I would write it:

#include <stdio.h>

float convert_dollars_to_pounds(float dollars);

int main(void)
{
    float dollars;
    float pounds;

    printf("Enter a dollar amount: $");
    scanf("%f", &dollars);

    pounds = convert_dollars_to_pounds(dollars);

    printf("That's equal to %.2f pounds!\n", pounds);

    return 0;
}

float convert_dollars_to_pounds(float dollars)
{
    return dollars*2;
}