Seg Fault Help

[sedition]

Hello all,

I'm in the process of of rewriting a combat section of existing mud code. I have experience in numerous other (mainly scripting) languages, but I'm trying to learn C on the fly during this process. Below is my code that prompts for a rank, calls a roll function (returns a result 1-100), and then utilize both results for an additonal function that determines a result (color) based upon the rank and the roll. After learning that I cannot readily return a string from a function the result is my below code. There a two bugs I'd like to have help in solving whether it be an instant fix, or even a recommendation for something to go read and learn how to solve on my own.

1. No matter if it's a pass or fail, the code always works, but results in a seg fault (though it compiles on gcc 4.3.3 without a hitch).
2. Occasionally, the code will process and prompt for rank to be entered twice. On the initial run, it processes and returns a result with no seg fault, then it runs again, processes, displays the result and seg faults.

Thanks in advance, here is the code:

#include <stdio.h>

char * act(char rank, int roll, char result[6]);

int main (void){
	int roll;
	char rank;
	char result[6];
	printf("Enter rank: ");
	scanf("%s", &rank);
	printf("Rank: %s...\n", &rank);
	roll = roll_hundred(0);
	printf("\tRoll: %d. Result: %s.\n", roll, act(rank,roll,result));
	return 0;
}

int roll_hundred(int mod){
	int roll, stime;
	srand (time(NULL));
	roll = rand() % 100 + 1;
	roll = roll + mod;
	return(roll);
}

char * act(char rank, int roll, char result[]){
	if(rank=='A'){
		if(roll>=100){result="Red";}
		else if(roll>=99){result="Green";}
		else if(roll>=74){result="Blue";}
		else if(roll>=18){result="White";}
		else{result="Black";}
	}
	else if(rank=='1'){
		if(roll>=100){result="Red";}
		else if(roll>=99){result="Orange";}
		else if(roll>=98){result="Yellow";}
		else if(roll>=96){result="Green";}
		else if(roll>=71){result="Blue";}
		else if(roll>=15){result="White";}
		else{result="Black";}
	}
	else{
		result="Rank not found";
	}
	return(result); 
}

[msh]

How many elements can result hold and how many elements are in "Rank not found" string literal?

Why is act() returning anything at all?

[auralius]

check variable rank please... it is only a single char, the problem is stack aroun this char might be corrupted....

[msh]

check variable rank please... it is only a single char, the problem is stack aroun this char might be corrupted....

Also true.

You're reading into rank as if it were a string. Strings are null-terminated. So how many characters are you really trying to stuff into rank even if you only input a single character? And no, you do not want to make char rank into char rank[2].

[sedition]

sorry, msh, I had originally pasted the wrong error message into my code. It should have been "Error", rather than "Rank not found".

Also true.

You're reading into rank as if it were a string. Strings are null-terminated. So how many characters are you really trying to stuff into rank even if you only input a single character? And no, you do not want to make char rank into char rank[2].

Followed auralius' tip and changed rank to char rank[3] as there will possibly double-digits input for the string rank. I was only submitting 1 character into the rank string but, the null character may have been throwing everything off. Everything seems copacetic now, thanks to you both.

Out of curiosity, msh, why would changing char rank to char rank[2] be a bad approach?

[Elysia]

You can't assign string literals to arrays. Use strcpy. It works because you essentially assigning a string literal to a temporary copy (a pointer to) your result array; but the actual result is NOT stored in the buffer you pass in.
Furthermore, you are trying to enter 6 characters to the result variable at times, but you only have room for 5 (+ null char)!

[msh]

Out of curiosity, msh, why would changing char rank to char rank[2] be a bad approach

I assumed that you said what you meant and rank is actually a single-character input.

If it is not so, disregard that comment.

rank=='A'

The above does not do what you think it does. What type is rank?

Read up on string library.
Compile with -Wall option. This will also make the compiler give you warnings.

[auralius]

if you really want to make it simple, why not changing this:

	scanf("%s", &rank);

to become:

	rank = getchar();

scanf will create null terminated array of chars...

[nynicue]

printf("Rank: %s...\n", &rank);

Are you sure & needs to be there?

[sedition]

Are you sure & needs to be there?

No, it doesn't. It has been removed.

The above does not do what you think it does. What type is rank?

Read up on string library.
Compile with -Wall option. This will also make the compiler give you warnings.

rank needs to be a string. Thanks for the link, I'm definitely gonna have to read up on this. I'm floundering on pointers and strings.

Last question, I think:
Is there a function that will trim null characters off an user-entered array (rank), or is it understood it will be there and checked for in comparisons, like if (rank='A')? Also, here is the new code in case it may help any other newcomers to c out:

#include <stdio.h>
#include <string.h> /* strcpy */

char * act(char rank[3], int roll, char result[7]);

int main (void){
	int roll;
	char rank[3];
	char result[7];
	printf("Enter rank: ");
	scanf("%2s", rank); /* %2s limits the allowed input and prevent buffer overflows */
	printf("Rank: %s...\n", rank);
	roll = roll_hundred(0);
	printf("\tRoll: %d. Result: %s.\n", roll, act(rank,roll,result));
	return 0;
}

int roll_hundred(int mod){
	int roll, stime;
	srand (time(NULL));
	roll = rand() % 100 + 1;
	roll = roll + mod;
	return(roll);
}

char * act(char rank[], int roll, char result[]){
	printf("lol: %s", rank);
if(rank=="A"){
	if(roll>=100){strcpy(result, "Red");}
	else if(roll>=99){strcpy(result, "Green");}
	else if(roll>=74){strcpy(result, "Blue");}
	else if(roll>=18){strcpy(result, "White");}
	else{strcpy(result, "Black");}
	}
else if(rank=="1"){
	if(roll>=100){strcpy(result, "Red");}
	else if(roll>=99){strcpy(result, "Orange");}
	else if(roll>=98){strcpy(result, "Yellow");}
	else if(roll>=96){strcpy(result, "Green");}
	else if(roll>=71){strcpy(result, "Blue");}
	else if(roll>=15){strcpy(result, "White");}
	else{strcpy(result, "Black");}
	}
else if(rank=="11"){
	if(roll>=98){strcpy(result, "Red");}
	else if(roll>=93){strcpy(result, "Orange");}
	else if(roll>=83){strcpy(result, "Yellow");}
	else if(roll>=71){strcpy(result, "Green");}
	else if(roll>=49){strcpy(result, "Blue");}
	else if(roll>=5){strcpy(result, "White");}
	else{strcpy(result, "Black");}
	}
else{
	strcpy(result, "Error");
}
	return(result); 
}

Thanks again to everyone for being so helpful and responding so quickly!

[msh]

rank=="A"
rank=="1"
rank=="11"

This is not what you meant.

Your comparing memory addresses - rank is the address of first element of rank array - and is non-zero so true. Same is true for "A", "1", "11" and any string constant - they are internally represented as an anonymous array (not the actual term) and "A", for example, is just a pointer to the first element of the said anonymous array containing ['A', '\0'].

If you want to compare two strings, you must use strcmp(), e.g. strcmp(rank, "A").

[sedition]

Your comparing memory addresses - rank is the address of first element of rank array - and is non-zero so true. Same is true for "A", "1", "11" and any string constant - they are internally represented as an anonymous array (not the actual term) and "A", for example, is just a pointer to the first element of the said anonymous array containing ['A', '\0'].

If you want to compare two strings, you must use strcmp(), e.g. strcmp(rank, "A").

I had tried strcmp earlier, but abandoned it because it was yielding inconsistent results. Apparently, if I had I read the reference more thoroughly (0 = positive match), then everything would have been working.
Thanks again everyone. Msh, you're the man! It actually makes sense since you put it like that. I guess I need to take the time and read things more thoroughly than assume I know what I'm doing.