composing a string

Programming, for all ages and all languages.
mohammed
Member
Member
Posts: 93
Joined: Mon Jan 30, 2006 12:00 am
Location: Mansoura, Egypt

composing a string

Post by mohammed »

hi all i am trying to make a string so i can print it with the function puts(unsigned char *str)
that's my function

Code: Select all

gets(unsigned char *string)
{
char c;
int cntr = 0;
while(c=getch() != '\n')
{
string[cntr] = c;
cntr++
}
cntr++;
string[cntr] = '\0';

}
is there something wrong with this function ?????
Last edited by mohammed on Sun Sep 30, 2007 2:30 am, edited 1 time in total.
User avatar
bluecode
Member
Member
Posts: 202
Joined: Wed Nov 17, 2004 12:00 am
Location: Germany
Contact:

Post by bluecode »

First of all the function prototype is not C99 compliant. It should be:

Code: Select all

char *gets(char *s);
Second, do you know that getch() belongs to curses. In order to use that function you should initialize curses. See initscr or newterm for more information.
Third, the cntr++ after the while loop should not be there.
Fourth, why do you return the first character?
User avatar
Solar
Member
Member
Posts: 7615
Joined: Thu Nov 16, 2006 12:01 pm
Location: Germany
Contact:

Re: composing a string

Post by Solar »

mohammed wrote:is there something wrong with this function ?????

Code: Select all

$ gcc -Wall -Wextra -c test.c
test.c: 2: warning: return type defaults to 'int'
test.c: 5: warning: implicit declaration of function 'getch'
test.c: 5: warning: suggest parentheses around assignment used as truth value
test.c: 9: error: expected ';' before '}' token
Both c and cntr are completely redundant. I could show you how, but I rather suggest you learn about pointers before continuing.
Every good solution is obvious once you've found it.
mohammed
Member
Member
Posts: 93
Joined: Mon Jan 30, 2006 12:00 am
Location: Mansoura, Egypt

Post by mohammed »

first okay i changed the prototype scond getc() is my function that returns a char and working well i wrote it getch as i used to write in in C i am sorry
third where should it be ??
fourth i thought that i am just returning the pointer of the beginning of the string ...

Code: Select all

Both c and cntr are completely redundant. I could show you how, but I rather suggest you learn about pointers before continuing.
the pointer points to the place of the variable instead of dealing with the value you deal with the address of the value here our pointer points to a C string that should end with the terminate zero '\0'
i assure you that i know pointers show me how then i think you punished me enough in the last topic :D
User avatar
Alboin
Member
Member
Posts: 1466
Joined: Thu Jan 04, 2007 3:29 pm
Location: Noricum and Pannonia

Post by Alboin »

mohammed wrote:i assure you that i know pointers show me how then i think you punished me enough in the last topic :D
A pointer's address may be increased, like an integer's. For example:

Code: Select all

pointer++;
Increases the pointer forward to it's next address. (ie. based on it's type.) Moreover, the current value of the pointer may be set like:

Code: Select all

*pointer = value;
I believe it was the combination of these two things that Solar was referring to. I won't show you, but by using the above, you can remove the two aforementioned variables from your code.
C8H10N4O2 | #446691 | Trust the nodes.
DeletedAccount
Member
Member
Posts: 566
Joined: Tue Jun 20, 2006 9:17 am

Yea .. there are some errors

Post by DeletedAccount »

First of all there is a problem of buffer overlow and using gets is
not recommended .. Your implementation of gets doesnt seem to fix
the problem .In your programs if the user enters a very loooong
string ... The program will crash with Segmentation Fault core dumped...


so use 1) fgets -- use STDIN instead as the filename
or open () in a similar fashion ...
mohammed
Member
Member
Posts: 93
Joined: Mon Jan 30, 2006 12:00 am
Location: Mansoura, Egypt

Post by mohammed »

Code: Select all

char *gets(char *string)
{
char c;

while(c=getc() != '\n')
{
*string++ =  c;
}
*string++  = '\0';
}
then this function have no problems at all ??? it suppose to work fine ??
is it a policy here not to showany one a code ??? or what ???
i tried this in main function

Code: Select all

char *zeko;
char *medo = "fdsfs";
char m;
m=getc();
putc(m); //worked
puts(medo); worked

gets(zeko); 
puts(zeko); // didn't work : ( ( 
User avatar
bluecode
Member
Member
Posts: 202
Joined: Wed Nov 17, 2004 12:00 am
Location: Germany
Contact:

Post by bluecode »

mohammed wrote:is it a policy here not to showany one a code ??? or what ???
No, but we have the interest that you learn as much as possible from our answers. And for me (others see this different, e.g. Dex) learning has nothing to do with copy & pasting sourcecode.

About your problem: Pointers should point to something, right? But where does zeko point to? Read something about memory (de)allocation, the heap (or free store) and the like. Functions in the C library for that purpose are: malloc, calloc, realloc & free
mohammed
Member
Member
Posts: 93
Joined: Mon Jan 30, 2006 12:00 am
Location: Mansoura, Egypt

Post by mohammed »

then the problem is with my pointer that i am trying to pass to gets not with gets itself ???
then an allocate function will solve the problem i mean you tried that before
and it will never work without malloc function ??
User avatar
bluecode
Member
Member
Posts: 202
Joined: Wed Nov 17, 2004 12:00 am
Location: Germany
Contact:

Post by bluecode »

mohammed wrote:and it will never work without malloc function ??
You can define a local/global array of chars and pass a pointer to the first element of that array to your gets() function.
mohammed
Member
Member
Posts: 93
Joined: Mon Jan 30, 2006 12:00 am
Location: Mansoura, Egypt

Post by mohammed »

that tried the arrays before and it worked but that is not what i want i want to compose a string that all C functions can deal with in normal C you just type char *str;
and then
gets(str);
gets return the string that you typed in str without any allocation of any kind ...!!
pcmattman
Member
Member
Posts: 2566
Joined: Sun Jan 14, 2007 9:15 pm
Libera.chat IRC: miselin
Location: Sydney, Australia (I come from a land down under!)
Contact:

Post by pcmattman »

I've never seen

Code: Select all

char* str;
gets( str );
You must allocate space for str, otherwise it isn't pointing anywhere, and gets is writing to what is possibly not even physical memory.
User avatar
JamesM
Member
Member
Posts: 2935
Joined: Tue Jul 10, 2007 5:27 am
Location: York, United Kingdom
Contact:

Post by JamesM »

mohammed wrote:

Code: Select all

char *gets(char *string)
{
char c;

while(c=getc() != '\n')
{
*string++ =  c;
}
*string++  = '\0';
}
then this function have no problems at all ??? it suppose to work fine ??
is it a policy here not to showany one a code ??? or what ???
i tried this in main function

Code: Select all

char *zeko;
char *medo = "fdsfs";
char m;
m=getc();
putc(m); //worked
puts(medo); worked

gets(zeko); 
puts(zeko); // didn't work : ( ( 
1) 'zeko' is undefined. You are writing to duff memory somewhere. (you're bloody lucky it isn't segfaulting). By the way, is this in a linux environment or in your own OS? (I ask because of all the time you spent getting a 'getc' function to work...)

2) As mentioned, your function is not overflow-safe. The function does not know anything about the maximum size of the input buffer, and as such can overflow. A safer prototype would be:

Code: Select all

char *gets(char *buffer, unsigned int max_length)
3) Your function prototype says it returns a "char *", yet there is no return statement.

4) the '++' part on '*string++ = '\0'' is redundant, you are not using string any more so there is no need for the extra increment.

5) personally I would parathesise the "c=getc()" expression to ease readablilty. It is a moot point, as because of operator prescdence it will work anyway, but I tend not to trust operator prescendence too much.

6) Please learn more about pointers.
mohammed wrote:that tried the arrays before and it worked but that is not what i want i want to compose a string that all C functions can deal with in normal C you just type char *str;
and then
gets(str);
gets return the string that you typed in str without any allocation of any kind ...!!
Completely and utterly incorrect. Allocation must be made somewhere, be it in the calling function or the callee. It is safer to allocate in the calling function as, in such cases, stack allocation can be used. However, if you want to do callee allocation:

Code: Select all

#define BUFSZ 512
char *gets()
{
  char *buffer = (char*)malloc(BUFSZ);
  unsigned int sz = 0;
  while( (*buffer++=getc()) != '\n' &&
         sz < BUFSZ-1)
    sz++;
  *--buffer  = '\0';
  return buffer;
}

... in main.c ...

char *string = gets();
free(string);
but note the static allocation size! this is not the best way to do this! (also note that I fixed your code ;) )
JamesM
Last edited by JamesM on Mon Oct 01, 2007 3:53 am, edited 1 time in total.
User avatar
Combuster
Member
Member
Posts: 9301
Joined: Wed Oct 18, 2006 3:45 am
Libera.chat IRC: [com]buster
Location: On the balcony, where I can actually keep 1½m distance
Contact:

Post by Combuster »

Actually, I'd consider it unlucky that it doesn't segfault...
"Certainly avoid yourself. He is a newbie and might not realize it. You'll hate his code deeply a few years down the road." - Sortie
[ My OS ] [ VDisk/SFS ]
mohammed
Member
Member
Posts: 93
Joined: Mon Jan 30, 2006 12:00 am
Location: Mansoura, Egypt

Post by mohammed »

sorry it should be char zeko[20];
working in bran's kernel so there will never be any segfault cause this pointer will be in the uninitialized data in .bss section ....(i think the linker script control both the .c files and asm files as well )right ? :?
but note the static allocation size! this is not the best way to do this! (also note that I fixed your code Wink )
JamesM
you are my best friend right here JamesM > : D < even my getc() function based on what you instructed me ...i mean about testing the 0x64 register
i don't have any malloc or free functions yet i am gonna writ....aaa... copy it to try your code and i will tell you okay ?
Post Reply