why is this crashing?

[EscapeCharacter]

i cant figure this out for the life of me, after the text is input is dies.

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

int main(){
char *limbo;
unsigned int x, y, z;
printf("Enter the phrase to be encrypted: ");
scanf("%s", limbo);
printf("%s", limbo);
char *temp;
unsigned int numtemp = 0;
unsigned int blah= strlen(limbo);
for(x = 0; x< blah; x+=4)
temp[numtemp++]=limbo[x];
for(y = 1; y< blah; y+=2)
temp[numtemp++]=limbo[y];
for(z = 2; z< blah; z+=4)
temp[numtemp++]=limbo[z];
temp[strlen(limbo)]='\0';
printf("%s", temp);
return 0;
}

[Energon]

you need to initialize your pointers... I'm not familiar with the C method (malloc()), but in C++ it'd look like this:


char *limbo;
limbo = new char[255];


so it's a pointer that will hold 255 characters including the null byte should it have one... the way you're doing it, you don't have any valid memory to write to, so you're writing to junk memory and it's Segfaulting because of that...

[EscapeCharacter]

its always the little things :) thanks

[jemfinch]

In C, you'd do this:


#define LIMBOLEN 256
char limbo[LIMBOLEN];
char buf[13] = sprintf("%%%ds", LIMBOLEN);
scanf(buf, limbo);
printf("%s", limbo);


On a 64 bit system you'll need to raise the size of "buf" to 23 to be absolutely robust. And, of course, do the same thing you did for limbo with "temp". And note that in C, you can't declare variables except at the start of a block, which means you'll need to collect those variable declarations at the beginning of the function.

Jeremy

[TheLinuxDuck]

EscapeChar:

What they said. Actually, the way to allocate memory (using malloc, with 255 bytes) in C is:

char *limbo;
//
limbo=(char *)malloc(255);
if(limbo==NULL) {
perror("Couldn't allocate memory");
return 1;
}


And, just so you have a heads up on getting away from that nasty scanf, I'm going to share with you a qool trick for handling user input.

The reason that scanf is nasty is because there is no way for it to make sure that the user doesn't enter more data than the variable has been allocated (this is true of any variable, actually). Even if you allocated 255 chars to limbo, if the user enters 300, scanf will still assign that inputted data to the pointer, and it will overwrite whatever is in memory past that 255 bytes.

The best way (of which I know) to make sure that user input gets truncated is to use fgets, with stdin as the file handle. The code will also need to do two things. 1. Move the stdin location pointer to the end of any leftover input. 2. Remove the newline from the input (that fgets leaves):

void userInput(char *storageBuffer,const int stringLimit)
{
fgets(storageBuffer,stringLimit,stdin);
fseek(stdin,0L,SEEK_END);
storageBuffer[strlen(storageBuffer)-1]='\0';
}

As long as storageBuffer has already been malloc'd, then this input routine works nicely.

Of course, the function can also including the malloc inside of it, too.. but then the code will have to has a way to deal with it if it fails to alloc the memory.. (^=

In C, it's all about the little things.

Dat's why C is fun!

[jemfinch]

Originally posted by TheLinuxDuck:
The reason that scanf is nasty is because there is no way for it to make sure that the user doesn't enter more data than the variable has been allocated (this is true of any variable, actually). Even if you allocated 255 chars to limbo, if the user enters 300, scanf will still assign that inputted data to the pointer, and it will overwrite whatever is in memory past that 255 bytes.


scanf does have a way to limit that input. If you put a number before the "%s", it'll read at most that number (minus one) and put a null character at the end. That's the point of my code above:


char buf[13] = sprintf("%%%ds", LIMBOLEN);


That creates the format string for the scanf under it dynamically, so LIMBOLEN is only hardcoded once. buf is made 13 bytes long because the maximum length of a 32bit number is 10 digits; plus 2 for the '%' and the 's', and another for the '\0' to terminate the string.

There aren't buffer overflows in the code I posted, because scanf was used properly.


storageBuffer[strlen(storageBuffer)-1]='\0';


strlen walks the string until it reaches a '\0'. Thus, a program using the above line will either set a null byte to a null byte (effectively doing nothing except wasting some time marching the string) or will walk off the edge of a non-null-terminated string, causing a segmentation fault at some point if it reads too many characters before finding a null byte.

It's all a moot point anyway because fgets returns a null-terminated string.

In C, it's all about the little things. That's why I use higher level languages :D

(I hate these C posts, actually...I never feel quite as sure of myself as I do with other languages...)

Jeremy

[TheLinuxDuck]

I guess I should have known about the scanf #, since it uses the same format as printf... (^=

I do have a question, though.. in the:

char buf[13] = sprintf("%%%ds", LIMBOLEN);

:line, according to that string, won't buf = %%256s ? Shouldn't that be %256s ? Or is that extra % necessary for that?

I can't even get a simple example with that line in it to compile.. it's telling me that it's trying to make a pointer from an integer.. I don't know that technique well enough to understand why it's giving me the error.

<STRONG>
Thus, a program using the above line (refering to line of code using strlen) will either set a null byte to a null byte (effectively doing nothing except wasting some time marching the string) or will walk off the edge of a non-null-terminated string, causing a segmentation fault at some point if it reads too many characters before finding a null byte.
</STRONG>

No offense, but that is not correct. Using that input method, the inputted string will always be null terminated from fgets (unless the programmer doesn't assign enough storage space for the string space in the first place, and there's no way that I know of to determine how much space was assigned to a malloc'd pointer based only on the pointer).

Strlen returns the size (in slices/elements) of the array. That number will always be 1 greater than the actual last slice of the array. fgets always leaves the carriage return on the string (**see below).

Take the following code example:

#include &lt;stdio.h&gt;
//
int main(void) {
char string1[256],string2[256];
//
printf("Press the enter key only:\n");
fgets(string1,256,stdin);
//
printf("Type in a few characters and press enter:\n");
fgets(string2,256,stdin);
//
printf("The string length of string1 is %d\n"
"The string length of string2 is %d\n", strlen(string1),
strlen(string2));
//
printf("The character at position 'strlen(string1)-1' of each string:\n"
" string1: (%d) %c\n"
" string2: (%d) %c\n",string1[strlen(string1)-1],
string1[strlen(string1)-1],
string2[strlen(string2)-1],
string2[strlen(string2)-1]);
return 0;
}


Take a look at it's output:

/home/root/c/tld&gt; ./fgets-ex
Press the enter key only:

Type in a few characters and press enter:
qwerty
The string length of string1 is 1
The string length of string2 is 7
The character at position 'strlen(string1)-1' of each string:
string1: (10)

string2: (10)



You see that the second to last slice of each string is a newline, which is what that line is removing.

**That is, unless the length of the input from stdin is greater than the amount that fgets has been told to allow.

There is one flaw with my input routine, and it is that it should be checking the second to last character against a newline, in case the user did enter more data than the input function will allow.. that way it will preserve that second to last character without truncating it.

Therefore, the userInput function *should* be:

void userInput(char *storageBuffer,const int stringLimit)
{
int len;
fgets(storageBuffer,stringLimit,stdin);
fseek(stdin,0L,SEEK_END);
len=strlen(storageBuffer)-1;
if(storageBuffer[len]=='\n') storageBuffer[len]='\0';
} }


That fixes the mistake. (^=

[TheLinuxDuck]

Err... minus that extra right-bracket at the end there.. (^=

[andrzej]

Mistaken you are, TLD.
strlen just searches given memory location for '\0' while incrementing a counter.
jemfinch was right (as he usually is :) )

[TheLinuxDuck]

Originally posted by andrzej:
<STRONG>Mistaken you are, TLD.
strlen just searches given memory location for '\0' while incrementing a counter.
jemfinch was right (as he usually is :) )</STRONG>

Mistaken? How? Look at the example code I posted above, and run it yourself. It will prove what I said.

Show me some example code I can run that will prove me wrong.

[edit]

I have been wrong before, and am not afraid to admit when I am wrong, but I don't see how that is wrong, especially since I have provided a working example that proves exactly what I have stated.

[ 03 August 2001: Message edited by: TheLinuxDuck ]

[andrzej]

main() {

char c[23];
c[6] = '\0';
printf("%i\n",strlen(c));
c[3] = '\0';
printf("%i\n",strlen(c));
return 0;
}

Output:
6
3

[TheLinuxDuck]

Originally posted by andrzej:
<STRONG>main() {

char c[23];
c[6] = '\0';
printf("%i\n",strlen(c));
c[3] = '\0';
printf("%i\n",strlen(c));
return 0;
}

Output:
6
3</STRONG>

You have proved me correct. Look at what I said:
From TheLinuxDuck<STRONG>
Strlen returns the size (in slices/elements) of the array.</STRONG>

c has been allocated, in your example to 23. The first time, it puts the null terminater at 6. That means that the 'string' is in positions 0,1,2,3,4 and 5. Add those, and the answer is 6, which is exactly what your example shows.

<STRONG>
That number will always be 1 greater than the actual last slice of the array. </STRONG>

If the strlen is 6, that means that the actual last slice of that array is in 5. That is 1 minus the number that strlen returns.

--

I get the feeling that we're misunderstanding each other somehow, but I cannot see what way it is.

[EscapeCharacter]

since you guys are having soo much fun with this, how would i go about making a variable sized limbo, as in changing its max size for whatever input it gets. say if the max buffer for is set to 255 but the input enter is 256, is it possible to change the size to include the extra char without over running the buffer?

[andrzej]

:)
You've written that jemfinch was wrong when he wrote


Thus, a program using the above line (refering to line of code using strlen) will either set a null byte to a null byte (effectively doing nothing except wasting some time marching the string) or will walk off the edge of a non-null-terminated string, causing a segmentation fault at some point if it reads too many characters before finding a null byte.

He was right. The line was:

storageBuffer[strlen(storageBuffer)-1] = '\0';

Assume storageBuffer is null terminated: strlen(storageBuffer) will return the number of characters before terminating '\0', so the above line will add a '\0' just before where it previously was. (he was almost right :o )

Now assume storageBuffer is not null terminated - strlen will go out of allocated memory in a search for '\0', will finally find one somewhere and try to write one character before - probably causing a segfault.

I don't try to comment on your code. I believe it works. (especially if fgets null-terminates the buffer, which it does).

I hope we begin to understand each other ;)

[andrzej]

:o

The time has come to apologize.

I've read through the whole thread, you're right.

:o

I should be more careful before posting. Sorry.

[kmj]

jemfinch is wrong; you're right Duck. (With the corrected code, verifying that it is actually a newline you're removing).. forgive us little turds for doubting you.

[TheLinuxDuck]

(^= I appreciate it.

I mistakenly said "size of the array" when in fact I meant "the number of valid chars in the string".

I was confusing "array of chars" and "null-terminated string" (as pointed out to me by kmj).

I apologize for that.

(^=

I feel that I should say that I, also, did not mean to imply that jemfinch was wrong about how strlen achieves it's return value.

I feel better, only because the confusion has been cleared.

My stomach was in knots (and still kinda is). (^=

[TheLinuxDuck]

Originally posted by EscapeCharacter:
<STRONG>since you guys are having soo much fun with this, how would i go about making a variable sized limbo, as in changing its max size for whatever input it gets. say if the max buffer for is set to 255 but the input enter is 256, is it possible to change the size to include the extra char without over running the buffer?</STRONG>

I can think of two ways to do this. Both would work better putting the malloc inside of the userInput function.

1. Use a predefined char array inside the userInput function, something large enough to handle just about anything that the user might throw at it.. say 8192 chars or so. Then, fgets into it instead of passing in a pointer to use. Once it is done, then malloc the new buffer with the strlen of the predefined char array, copy it over, then return a pointer to that new char array. That way, the predefined buffer disappears once that function is done and the new string is passed out, at the correct size.

Hm.. I thought I had another way, but I'm still mulling it over in my head.. let me play and see what I can come up with. Maybe someone else can provide some way of doing so. I've never tried to do that before. (^=

[EscapeCharacter]

sweet thanks tld
/me needs to learn more about c functions :)

[pinoy]

#define LIMBOLEN 256
char limbo[LIMBOLEN];
char buf[13] = sprintf("%%%ds", LIMBOLEN);
scanf(buf, limbo);
printf("%s", limbo);


sprintf, takes a buffer, not return it. Like most write/printf functions sprintf returns the number of characters written.

And as for scanf, I also tend to avoid it. See comp.lang.c FAQ for why. http://www.eskimo.com/~scs/C-faq/q12.20.html

Also the sprintf is actually unnecessary from the above code if you use macros. Just use the 'stringify' operator. (I'm not sure if this is part of the standard though, I can't verify it as I've already packed up all my books).


#define LIMBOLEN 256
char limbo[LIMBOLEN];
char *format = "%" #LIMBOLEN "s"
scanf(format, limbo);
printf("%s", limbo);


strlen counts all characters up to the null terminator.

A strlen function can be easily implemented as:

size_t strlen(const char *s)
{
size_t len = 0;
for (; *s; s++, len++)
;
return len;
}


or

size_t strlen(const char *s)
{
const char *p = s;
while (*p)
p++;
return p - s;
}


I probably prefer the second one as it involves only incrementing one variable.


storageBuffer[strlen(storageBuffer)-1]='\0';


is basically chopping the last character off and is perfectly valid.


since you guys are having soo much fun with this, how would i go about making a variable sized limbo, as in changing its max size for whatever input it gets. say if the max buffer for is set to 255 but the input enter is 256, is it possible to change the size to include the extra char without over running the buffer?


This is complicated. Why not just allocate more than what you need. malloc 5 megs if you want. You want to allocate this, not declare it on the stack as stack space is usually limited. Otherwise you will have to keep track of a list of buffers. Much like editors do. I'm not saying this is the best solution, but until you get better with C, this is a good solution.

[jemfinch]

Yeah, TLD, you were right. I skimmed over your code and assumed you were putting the null character in the string to assure yourself of its null-terminatedness, not to replace the newline :)

To add another strlen implementation to LloydM's list, here's the current one from FreeBSD:


size_t
strlen(str)
const char *str;
{
register const char *s;

for (s = str; *s; ++s);
return(s - str);
}


And about sprintf, I guess I'm too used to languages that return a string from it, rather than taking a buffer (icky :))

Jeremy

[TheLinuxDuck]

Originally posted by jemfinch:
<STRONG>Yeah, TLD, you were right. I skimmed over your code and assumed you were putting the null character in the string to assure yourself of its null-terminatedness, not to replace the newline :)</STRONG>


I figured that there was some misunderstanding. Of course, I didn't explain myself well (about what that line was doing), so it certainly accounts for the misunderstanding.

[ 05 August 2001: Message edited by: TheLinuxDuck ]