You should get the "not all paths return a value" in gcc if you use "-Wall" as the compile flags.
And get_input does have ways of exiting without returning a value, never mind that you never CHECK the value in the calling code, so it will happily continue trying to read new data when it's hit an end of file.
--
Mats
Compilers can produce warnings - make the compiler programmers happy: Use them!
Please don't PM me for help - and no, I don't do help over instant messengers.
I use Visual Studio 2008, but the compiler is irrelevant. You're just not enabling all warnings.
Your functions returns a value of failure, but not on success, which is kindof bad. It should return a value on success to and you should probably check the return to see if it failed or not.
Originally Posted by Adak
Okay, I'm a newbie as I've said and I'm pretty damn pleased that the prog is working the way I want it to. I'm still a long way from knowing how to optimize or maximize safety or whatever but I am interested in learning this. Could tell me what you mean by "kindof bad"? What's the standard for checking returns for both success and failure? Cheers.
Last edited by SiliconHobo; 12-14-2007 at 07:26 PM.
Currently, your function is designed to return -1 on failure. However, upon success, it doesn't return anything.
Since you return something from your function, you need to put it somewhere in your main program; so you might have
If get_input fails, error_check will be -1. If get_input doesn't fail, though, error_check will be ... something. I don't know what, which makes it hard to check for.Code:error_check = get_input (prompt1, entry.name, sizeof(entry.name));
That's just it. It's undefined. Usually, return values are stored in the eax register (on x86), but if you don't return anything, the current value of the eax register will be interpreted as the return data, which is bad, of course.
Okay, so this is basically how loose ends are tied anytime a non-void function is called? I added:Suppose I change the call from input () to:Code:if (fgets_ptr==NULL) return -1 if (fgets_ptr!=NULL) return 1;
as you suggested. Then, what should I do with error_check? Also, if returning values for success and failure is generally good practice to increase safety, does that mean that void functions should be avoided or is it just of particular concern here because of fgets? Cheers.Code:error_check = get_input (prompt1, entry.name, sizeof(entry.name));
I suggest you add something like return 0; at the end of the function. If the function succeeds, the execution will eventually reach the end of the function so you can return 0 from there, to indicate success (or any other value of choice, like 1; it's up to you).
What you should do with error_check? You should check if the function succeeded or not. If it failed, then you probably won't be able to proceed because necessary data is missing.
IF to continue anyway, there's a good chance the program might crash! And that's a bad thing. Very bad thing. It's better to just pop up an error and abort.
If you can, all functions should return success or fail. If they cannot, then void is acceptable. You can also require the caller to pass in a variable to set if there was an error or not. In that case, the function could return void. Generally, though, functions return success or not. There's nothing wrong with either way, of course, but returning an error might be better because the caller doesn't have to pass in a variable. It can completely ignore your error you return too; it's up to context, really.
