Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > Something wrong in my program

Reply
Thread Tools

Something wrong in my program

 
 
Dominique =?ISO-8859-1?Q?L=E9ger?=
Guest
Posts: n/a
 
      01-16-2004
Hello guys,

I'm kinda new to C, and I'm having a hard time with strings. What I'm trying
to do is a simple function that trims spaces & tabs at the beginning of a
given string. For example, I want this: " Hello World" to become this:
"Hello World". At first glance, my function seems to work, but returns some
strange values...

Here's my code (please pardon the mess):

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

int main(void){
char *trimbegin(char *text);
char *str = " Hello World!";
char *result = trimbegin(str);
printf("What the function returns: \"%s\"\n", result);
return 0;
}

char *trimbegin(char *text){
int i = 0, j = 0, ok = 0;
int size = strlen(text);
char buffer[size + 1];
char *ptr;

printf("Original text is: \"%s\"\n", text);
printf("That's %d characters long...\n", size);
printf("Now, our text buffer can contain %d characters\n", size +
1);
for (i = 0; i <= size; i++){
if (ok == 1){
buffer[j] = text[i];
j++;
}
else if (isspace(text[i]) == 0 && ok == 0){
buffer[j] = text[i];
j++;
ok = 1;
}
}
printf("What the result is supposed to be: \"%s\"\n", buffer);
ptr = buffer;
return ptr;
}

And here's the output:

[dom@localhost C]$ ./a.out
Original text is: " Hello World!"
That's 14 characters long...
Now, our text buffer can contain 15 characters
What the result is supposed to be: "Hello World!"
What the function returns: "Hello World! @8@$÷ÿ¿¸öÿ¿PS@ý@Èöÿ¿"


Why does it return "Hello World! @8@$÷ÿ¿¸öÿ¿PS@ý@Èöÿ¿" and not "Hello
World!"?

Thanks for all your help!

-Dom

 
Reply With Quote
 
 
 
 
lallous
Guest
Posts: n/a
 
      01-16-2004
"Dominique Léger" <> wrote in message
news:...
> Hello guys,
>
> I'm kinda new to C, and I'm having a hard time with strings. What I'm

trying
> to do is a simple function that trims spaces & tabs at the beginning of a
> given string. For example, I want this: " Hello World" to become this:
> "Hello World". At first glance, my function seems to work, but returns

some
> strange values...
>
> Here's my code (please pardon the mess):
>
> #include <stdio.h>
> #include <string.h>
>
> int main(void){
> char *trimbegin(char *text);
> char *str = " Hello World!";
> char *result = trimbegin(str);
> printf("What the function returns: \"%s\"\n", result);
> return 0;
> }
>
> char *trimbegin(char *text){
> int i = 0, j = 0, ok = 0;
> int size = strlen(text);
> char buffer[size + 1];
> char *ptr;
>
> printf("Original text is: \"%s\"\n", text);
> printf("That's %d characters long...\n", size);
> printf("Now, our text buffer can contain %d characters\n", size +
> 1);
> for (i = 0; i <= size; i++){
> if (ok == 1){
> buffer[j] = text[i];
> j++;
> }
> else if (isspace(text[i]) == 0 && ok == 0){
> buffer[j] = text[i];
> j++;
> ok = 1;
> }
> }
> printf("What the result is supposed to be: \"%s\"\n", buffer);
> ptr = buffer;
> return ptr;
> }
>
> And here's the output:
>
> [dom@localhost C]$ ./a.out
> Original text is: " Hello World!"
> That's 14 characters long...
> Now, our text buffer can contain 15 characters
> What the result is supposed to be: "Hello World!"
> What the function returns: "Hello World! @8@$÷ÿ¿¸öÿ¿PS@ý@Èöÿ¿"
>
>
> Why does it return "Hello World! @8@$÷ÿ¿¸öÿ¿PS@ý@Èöÿ¿" and not "Hello
> World!"?
>
> Thanks for all your help!
>
> -Dom
>

Hello, guess you forgot to add a zero terminator to the string.

From the code, it would be something like: buffer[j] = 0 just right before
the printf() statement.

> int size = strlen(text);
> char buffer[size + 1];

I wonder how is this accepted by a C compiler?

--
Elias


 
Reply With Quote
 
 
 
 
Christopher Benson-Manica
Guest
Posts: n/a
 
      01-16-2004
Dominique Léger <> spoke thus:

> char *trimbegin(char *text){
> char buffer[size + 1];
> char *ptr;
> /* intervening code trimmed */
> ptr = buffer;
> return ptr;

^^^^^^^^^^ returns the address of buffer! (clc nitpicks
on that statement welcome!)
> }


Surprisingly (at least to you ), this is all the code you need to
see what's going wrong. You are returning the address of an automatic
variable, and when trimbegin returns, the memory that was reserved for
buffer (and that ptr pointed to, and that you returned a pointer to)
is freed. Thus, that memory is no longer guaranteed to be good for
anything, and you should not use it. If you did something like

ptr=malloc( strlen(buffer)+1 ); /* sizeof( char ) guaranteed to be 1 */
strcpy( ptr, buffer );
return ptr;

you'd be fine.

--
Christopher Benson-Manica | I *should* know what I'm talking about - if I
ataru(at)cyberspace.org | don't, I need to know. Flames welcome.
 
Reply With Quote
 
Joona I Palaste
Guest
Posts: n/a
 
      01-16-2004
Christopher Benson-Manica <> scribbled the following:
> Dominique Léger <> spoke thus:
>> char *trimbegin(char *text){
>> char buffer[size + 1];
>> char *ptr;
>> /* intervening code trimmed */
>> ptr = buffer;
>> return ptr;

> ^^^^^^^^^^ returns the address of buffer! (clc nitpicks
> on that statement welcome!)


Actually it returns the address of a char, which corresponds to the
exact same machine-level address as the address of buffer. The only
difference between them is the type.

>> }


--
/-- Joona Palaste () ------------- Finland --------\
\-- http://www.helsinki.fi/~palaste --------------------- rules! --------/
"It's not survival of the fattest, it's survival of the fittest."
- Ludvig von Drake
 
Reply With Quote
 
Christopher Benson-Manica
Guest
Posts: n/a
 
      01-16-2004
Christopher Benson-Manica <> spoke thus:

> If you did something like
> (blah)
> you'd be fine.


Well, that and listened (probably) to a previous poster who wondered why you
were able to compile

char buffer[size+1]; /* dynamic size arrays are C99, yes? */

--
Christopher Benson-Manica | I *should* know what I'm talking about - if I
ataru(at)cyberspace.org | don't, I need to know. Flames welcome.
 
Reply With Quote
 
Irrwahn Grausewitz
Guest
Posts: n/a
 
      01-16-2004
Dominique Léger <> wrote:
>Hello guys,
>
>I'm kinda new to C, and I'm having a hard time with strings. What I'm trying
>to do is a simple function that trims spaces & tabs at the beginning of a
>given string. For example, I want this: " Hello World" to become this:
>"Hello World". At first glance, my function seems to work, but returns some
>strange values...
>
>Here's my code (please pardon the mess):
>
>#include <stdio.h>
>#include <string.h>


#include <ctype.h> /* for isspace */

>int main(void){
> char *trimbegin(char *text);

Function prototypes shouldn't be buried in function definitions,
IMO. Move the prototype outside main, or define trimbegin before
main.

> char *str = " Hello World!";
> char *result = trimbegin(str);
> printf("What the function returns: \"%s\"\n", result);
> return 0;
>}
>
>char *trimbegin(char *text){
> int i = 0, j = 0, ok = 0;
> int size = strlen(text);
> char buffer[size + 1];
> char *ptr;
>
> printf("Original text is: \"%s\"\n", text);
> printf("That's %d characters long...\n", size);
> printf("Now, our text buffer can contain %d characters\n", size +
>1);
> for (i = 0; i <= size; i++){
> if (ok == 1){
> buffer[j] = text[i];
> j++;
> }
> else if (isspace(text[i]) == 0 && ok == 0){
> buffer[j] = text[i];
> j++;
> ok = 1;
> }
> }


That's a very complicated way to do the job at hand.

> printf("What the result is supposed to be: \"%s\"\n", buffer);
> ptr = buffer;


Obfuscation is not a cure for undefined behaviour...

> return ptr;


... which you invoke here by returning an invalid address, since
buffer will be gone when control reaches end of function.
Either
- qualify buffer static (rendering the function non-reentrant
and making C99 VLAs impossible to use), or
- provide a second parameter and let the caller provide the
buffer, or
- dynamically allocate memory for buffer in the function and
document that the caller has to free the memory if it's no
longer needed, or
- return only a pointer to the first non-whitespace character
in the original string,
whatever fits your needs best.

The last one is particularly easy to implement:

char *skipspace( char *s )
{
while ( isspace( *s ) )
s++;
return s;
}

>}
>
>And here's the output:
>
>[dom@localhost C]$ ./a.out
>Original text is: " Hello World!"
>That's 14 characters long...
>Now, our text buffer can contain 15 characters
>What the result is supposed to be: "Hello World!"
>What the function returns: "Hello World! @8@$÷ÿ¿¸öÿ¿PS@ý@Èöÿ¿"
>
>
>Why does it return "Hello World! @8@$÷ÿ¿¸öÿ¿PS@ý@Èöÿ¿" and not "Hello
>World!"?


You were lucky, because you invoked undefined behaviour, didn't
get what you expected (indicating that something's wrong) and
nothing really serious happened.

HTH

Regards
--
Irrwahn Grausewitz ()
welcome to clc : http://www.ungerhu.com/jxh/clc.welcome.txt
clc faq-list : http://www.eskimo.com/~scs/C-faq/top.html
acllc-c++ faq : http://www.contrib.andrew.cmu.edu/~a...FAQ-acllc.html

 
Reply With Quote
 
Bruno Desthuilliers
Guest
Posts: n/a
 
      01-16-2004
Dominique Léger wrote:
> Hello guys,
>
> I'm kinda new to C, and I'm having a hard time with strings. What I'm trying
> to do is a simple function that trims spaces & tabs at the beginning of a
> given string. For example, I want this: " Hello World" to become this:
> "Hello World". At first glance, my function seems to work, but returns some
> strange values...
>
> Here's my code (please pardon the mess):
>
> #include <stdio.h>
> #include <string.h>
>
> int main(void){
> char *trimbegin(char *text);


Move the function definition above the main(), and you won't have to
declare its prototype here.

> char *str = " Hello World!";
> char *result = trimbegin(str);
> printf("What the function returns: \"%s\"\n", result);
> return 0;
> }
>
> char *trimbegin(char *text){
> int i = 0, j = 0, ok = 0;
> int size = strlen(text);

should be a size_t, not an int.

> char buffer[size + 1];

gcc -Wall -ansi -pedantic :
warning: ISO C89 forbids variable-size array `buffer'


> char *ptr;
>
> printf("Original text is: \"%s\"\n", text);
> printf("That's %d characters long...\n", size);
> printf("Now, our text buffer can contain %d characters\n", size +
> 1);
> for (i = 0; i <= size; i++){
> if (ok == 1){
> buffer[j] = text[i];
> j++;
> }
> else if (isspace(text[i]) == 0 && ok == 0){

gcc -Wall -ansi -pedantic :
warning: implicit declaration of function `isspace'

> buffer[j] = text[i];
> j++;
> ok = 1;
> }
> }


Are you sure your algorithm is ok and as simple as it could be ?

> printf("What the result is supposed to be: \"%s\"\n", buffer);
> ptr = buffer;
> return ptr;


Which can be shortened to :
return &buffer[0];
and further to :
return buffer;

in which case you've got an additionnal, and very annoying warning :

gcc -Wall -ansi -pedantic :
warning: function returns address of local variable

> }


You understand that, even if the value returned is an effective memory
address, what becomes of the memory block starting at this address is no
more under your control as soon as the function returns ? This memory
may be reclaimed and reused by the system at any time, so trying to read
at this at address may have any unpredictable result - not talking about
*writing* at this address.

> And here's the output:
>
> [dom@localhost C]$ ./a.out
> Original text is: " Hello World!"
> That's 14 characters long...
> Now, our text buffer can contain 15 characters
> What the result is supposed to be: "Hello World!"
> What the function returns: "Hello World! @8@$÷ÿ¿¸öÿ¿PS@ý@Èöÿ¿"
>
>
> Why does it return "Hello World! @8@$÷ÿ¿¸öÿ¿PS@ý@Èöÿ¿" and not "Hello
> World!"?


First you forgot to copy the terminating '\0' to buffer, so buffer is
char array, but not a string. In C, a string is a char array *terminated
by the char '\0'*. Note that this character is *not* counted by
strlen(), since it's not part of the string, but merely a 'sentinel'
indicating where the string ends (the 'effective' string may be shorter
than the memory block it is contained in).

Then, you return the address of a local automatic variable, which
invokes UB (Undefined Behavior) as soon as you read or write at this
address. Anything can happen, even that it *seems* to work correctly.

Solutions are :
- either dynamically allocate memory for buffer in trimbegin(), or
declare buffer outside trimbegin() and pass its address to trimbegin
(for now you'd better try the second solution)
- make sure that buffer terminates with a '\0' (the simplest way to do
it being to copy the terminating '\0' of the source string)
- eventually rewrite your algorithm to make it simpler

tip 1 : you dont need any test in the for loop body nor the ok flag
tip 2 : the for loop syntax is :
for (<initialisation>;<test>;<on_each_iteration>)
{
<body>
}
where any of <initialisation>, <test>, <on_each_iteration>, and
<body> can be empty.


Feel free to post amended code for review !-)

HTH
Bruno

 
Reply With Quote
 
Christopher Benson-Manica
Guest
Posts: n/a
 
      01-16-2004
"Régis Troadec" <> spoke thus:

> int main()
> {
> char * str = " \tHello World";
> char * trimmed;
> printf("Original :\n%s\n",str);
> trimmed = trim(str);
> printf("Trimmed :\n%s\n",trimmed);

/* What will happen if trim fails (returns NULL)? Of course,
it never does so at the moment, but if it were coded
correctly it might... */
> free(trimmed);
> }


> char* trim(char* s)
> {
> char * res;
> int cnt = 0;
> int cnt2 = 0;
> while (isspace(s[cnt]) != 0)
> cnt++;
> res = (char*)malloc(sizeof(char)*(strlen(s)-cnt+1));

/* There are numerous reasons why casting the return value of
malloc() is inadvisable, although I will let others
enumerate them */
/* And of course, what if malloc() fails? */
> while (s[cnt] != '\0')
> res[cnt2++] = s[cnt++];
> res[cnt2] = '\0';
> return &res[0];

/* res===&res[0], so why obfuscate? */
> }


I don't claim that all is well otherwise, but those are what my
untrained eyes see.

--
Christopher Benson-Manica | I *should* know what I'm talking about - if I
ataru(at)cyberspace.org | don't, I need to know. Flames welcome.

(Ignore blank lines following - need to keep tin happy)









 
Reply With Quote
 
Régis Troadec
Guest
Posts: n/a
 
      01-16-2004
Hi,

"Dominique Léger" <> a écrit dans le message de news:
...
> Hello guys,
>
> I'm kinda new to C, and I'm having a hard time with strings. What I'm

trying
> to do is a simple function that trims spaces & tabs at the beginning of a
> given string. For example, I want this: " Hello World" to become this:
> "Hello World". At first glance, my function seems to work, but returns

some
> strange values...
>
> Here's my code (please pardon the mess):
>
> #include <stdio.h>
> #include <string.h>


#include <ctype.h> /* for isspace()*/

> int main(void){
> char *trimbegin(char *text);
> char *str = " Hello World!";
> char *result = trimbegin(str);
> printf("What the function returns: \"%s\"\n", result);
> return 0;
> }
>
> char *trimbegin(char *text){
> int i = 0, j = 0, ok = 0;
> int size = strlen(text);
> char buffer[size + 1];


VLA in C99 only

> char *ptr;
>
> printf("Original text is: \"%s\"\n", text);
> printf("That's %d characters long...\n", size);
> printf("Now, our text buffer can contain %d characters\n", size +
> 1);
> for (i = 0; i <= size; i++){
> if (ok == 1){
> buffer[j] = text[i];
> j++;
> }
> else if (isspace(text[i]) == 0 && ok == 0){
> buffer[j] = text[i];
> j++;
> ok = 1;
> }
> }


I'm not sure about your algorithm, since ok is initialized to 0 and that
isspace() returns a non-zero value if it is a white-space character.

> printf("What the result is supposed to be: \"%s\"\n", buffer);
> ptr = buffer;
> return ptr;
> }
>
> And here's the output:
>
> [dom@localhost C]$ ./a.out
> Original text is: " Hello World!"
> That's 14 characters long...
> Now, our text buffer can contain 15 characters
> What the result is supposed to be: "Hello World!"
> What the function returns: "Hello World! @8@$÷ÿ¿¸öÿ¿PS@ý@Èöÿ¿"
>
>
> Why does it return "Hello World! @8@$÷ÿ¿¸öÿ¿PS@ý@Èöÿ¿" and not "Hello
> World!"?
> Thanks for all your help!
>
> -Dom


I suggest you my example :

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

char* trim(char* s);

int main()
{
char * str = " \tHello World";
char * trimmed;
printf("Original :\n%s\n",str);
trimmed = trim(str);
printf("Trimmed :\n%s\n",trimmed);
free(trimmed);
}

char* trim(char* s)
{
char * res;
int cnt = 0;
int cnt2 = 0;

while (isspace(s[cnt]) != 0)
cnt++;

res = (char*)malloc(sizeof(char)*(strlen(s)-cnt+1));

while (s[cnt] != '\0')
res[cnt2++] = s[cnt++];

res[cnt2] = '\0';

return &res[0];
}

Best regards. régis


 
Reply With Quote
 
Bruno Desthuilliers
Guest
Posts: n/a
 
      01-16-2004
Régis Troadec wrote:
> Hi,
>
> "Dominique Léger" <> a écrit dans le message de news:
> ...
>
>>Hello guys,
>>
>>I'm kinda new to C, and I'm having a hard time with strings. What I'm

>
> trying
>
>>to do is a simple function that trims spaces & tabs at the beginning of a
>>given string. For example, I want this: " Hello World" to become this:
>>"Hello World". At first glance, my function seems to work, but returns

>
> some
>
>>strange values...
>>
>>Here's my code (please pardon the mess):

(snip OP code)
>
> I suggest you my example :


I suggest the small corrections below...

> #include <ctype.h>
> #include <stdio.h>
> #include <string.h>
> #include <stdlib.h>
>
> char* trim(char* s);
>
> int main()

int main(void) or int main(int argc, char **argv)
> {
> char * str = " \tHello World";
> char * trimmed;
> printf("Original :\n%s\n",str);
> trimmed = trim(str);
> printf("Trimmed :\n%s\n",trimmed);
> free(trimmed);
> }
>
> char* trim(char* s)
> {
> char * res;
> int cnt = 0;
> int cnt2 = 0;
>
> while (isspace(s[cnt]) != 0)
> cnt++;
>
> res = (char*)malloc(sizeof(char)*(strlen(s)-cnt+1));

res = malloc(strlen(s) - cnt + 1);
if (res != NULL) /* malloc may fail */
{
> while (s[cnt] != '\0')
> res[cnt2++] = s[cnt++];
>
> res[cnt2] = '\0';

}
else {
/* whatever */
}
> return &res[0];

return res;
> }


HTH
Bruno

 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
XPath query for <?define something="something" ?> Pekka Järvinen XML 2 04-29-2008 08:12 PM
How to find and replace something that is nested inside something else? alainfri@gmail.com Perl Misc 4 05-31-2007 11:50 PM
something wrong with my chatting program! Flamingo Java 2 07-11-2006 02:32 AM
umm... something... template(s)... something else... pointer(s)... and such... 0.o yah, I'm hopeless and clueless o.0 C++ 4 10-13-2004 10:34 PM
Add/Remove Program Glitch: Asks If I Want To To Remove Wrong Program ? Robert11 Computer Support 6 08-02-2004 09:02 PM



Advertisments