Re: Something wrong in my program
From: Dominique Léger (dleger_at_vif.com)
Date: 01/17/04
- Next message: Keith Thompson: "Re: error at compiling"
- Previous message: Ben Pfaff: "Re: error at compiling"
- In reply to: Bruno Desthuilliers: "Re: Something wrong in my program"
- Next in thread: Sidney Cadot: "Re: Something wrong in my program"
- Reply: Sidney Cadot: "Re: Something wrong in my program"
- Reply: Bruno Desthuilliers: "Re: Something wrong in my program"
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Date: Fri, 16 Jan 2004 18:51:45 -0500
Bruno Desthuilliers wrote:
> 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
Here's the amended code:
#include <stdio.h>
#include <string.h>
#include <ctype.h>
char *trim(const char *text);
int main(void){
char *result = trim(" Hello World!");
printf("What the function returns: \"%s\"\n", result);
return 0;
}
char *trim(const char *text){
int i = 0;
static char buffer[20];
while( isspace(text[i]) ) i++;
strcpy(buffer, &text[i]);
printf("What the result is supposed to be: \"%s\"\n", buffer);
return buffer;
}
A little better, isn't it? :-) However, dynamically allocating memory for
'buffer' would definitely be more interesting. I'll try to figure it out on
my own and post the results later...
I have one last question: Everybody here seems to despise the new standard,
C99. Why is that so?
-Dom
- Next message: Keith Thompson: "Re: error at compiling"
- Previous message: Ben Pfaff: "Re: error at compiling"
- In reply to: Bruno Desthuilliers: "Re: Something wrong in my program"
- Next in thread: Sidney Cadot: "Re: Something wrong in my program"
- Reply: Sidney Cadot: "Re: Something wrong in my program"
- Reply: Bruno Desthuilliers: "Re: Something wrong in my program"
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Relevant Pages
|
