void TrimCString(char *str)
{
// Trim whitespace from beginning:
size_t i = 0;
size_t j;
while(isspace(str[i]))
{
i++;
}
if(i > 0)
{
for(j = 0; i < strlen(str); j++, i++)
{
str[j] = str[i];
}
str[j] = '\0';
}
// Trim whitespace from end:
i = strlen(str) - 1;
while(isspace(str[i]))
{
i--;
}
if(i < (strlen(str) - 1))
{
str[i + 1] = '\0';
}
}
> void TrimCString(char *str)
> {
> // Trim whitespace from beginning:
I guess I would trim from the end first since then you have less
copying to do afterwards.
> size_t i = 0;
> size_t j;
> while(isspace(str[i]))
isspace() expects an int and not a char as it's argument.
> {
> i++;
> }
> if(i > 0)
> {
> for(j = 0; i < strlen(str); j++, i++)
> {
> str[j] = str[i];
> }
> str[j] = '\0';
> }
An alternative would be to use memmove() here, so you don't
have to do it byte by byte. Also callling strlen() each time
through the loop is a bit of a waste of time - it doesn't
change and can be replaced by a check if str[i] is '\0'.
> // Trim whitespace from end:
> i = strlen(str) - 1;
Careful: This could set 'i' to -1 (if the string consistet of white
space only) and then the rest won't work anymore.
> while(isspace(str[i]))
> {
> i--;
> }
> if(i < (strlen(str) - 1))
> {
> str[i + 1] = '\0';
> }
> }
Here's an alternative version using pointers (and trying to
minimize the number of calls of strlen()):
void
TrimCString( char *str )
{
char *p,
*q;
/* Check that we've got something that looks like a string */
if ( ! str || ! * str )
return;
/* Trim from end */
for ( p = str + strlen( str ) - 1; p != str && isspace( ( int ) *p ); p-- )
/* empty */ ;
if ( p == str ) /* only white space in string */
{
*str = '\0';
return;
}
*++p = '\0';
/* Trim from start */
for ( q = str; isspace( ( int ) *q ); q++ )
/* empty */ ;
if ( q != str )
memmove( str, q, p - q + 1 );
}
Regards, Jens
--
\ Jens Thoms Toerring ___ j...@toerring.de
\__________________________ http://toerring.de
Why not return a char *, like most other string functions?
char *TrimCString(char *str)
using char * enables you to piggyback your function in the
middle of other functions, eg
printf("%s\n", TrimCString(someString));
> {
> // Trim whitespace from beginning:
> size_t i = 0;
> size_t j;
>
> while(isspace(str[i]))
> {
> i++;
> }
> if(i > 0)
> {
> for(j = 0; i < strlen(str); j++, i++)
move the strlen() outside the loop.
Maybe use memmove() instead of the loop.
> {
> str[j] = str[i];
> }
> str[j] = '\0';
> }
>
> // Trim whitespace from end:
> i = strlen(str) - 1;
Use the strlen you computed before, when
you moved it out of the for loop above :)
>
> while(isspace(str[i]))
> {
> i--;
> }
> if(i < (strlen(str) - 1))
> {
> str[i + 1] = '\0';
No need for the test.
when i >= (strlen(str) - 1) -- it can only be equal, anyway -- the
assignment overwrites a '\0' with a brand new '\0'.
Anyway, if you want to keep the test, use the computed strlen.
> }
> }
A couple what-if's
* what if a pass NULL to the function?
TrimCString(NULL);
* what if I pass a constant string literal to the function?
TrimCString(" 4 spaces at both ends ");
This pair of eyes sees three bugs, two occurring twice each
and the other perhaps due to too much snippage. There are also
some opportunities to improve speed and style. So, here we go:
Bug: Since you're using isspace() and strlen(), you need to
#include <ctype.h> and <string.h> to get their declarations.
Without the declarations, a compiler operating under C99 rules
must generate a diagnostic. Under C89 rules the code will work,
but might not be as fast as if the vendor's "magic" declarations
were present.
> void TrimCString(char *str)
> {
> // Trim whitespace from beginning:
> size_t i = 0;
Style: Instead of initializing `i' at its declaration and then
relying on the initial value later on, consider initializing it
closer to its use. The `for' statement is convenient for such
purposes.
> size_t j;
>
> while(isspace(str[i]))
Bug: If `str' contains negative-valued characters, this use
may violate the "contract" of isspace() by passing an out-of-range
argument. Use `while (isspace( (unsigned char)str[i] ))'. (This
is one of those occasions where a cast is almost always required
and almost always omitted, as opposed to the more usual situation
where a cast is almost always inserted and almost always wrong.)
> {
> i++;
> }
> if(i > 0)
> {
> for(j = 0; i < strlen(str); j++, i++)
Speed: This loop calculates strlen(str) on every iteration.
Since it will return the same value each time, all calls after the
first are wasted effort. Call strlen() once before the loop and
store the result in a variable, and use that variable to control
the loop.
> {
> str[j] = str[i];
> }
> str[j] = '\0';
> }
>
> // Trim whitespace from end:
> i = strlen(str) - 1;
Bug: If `str' is the empty string (either because it was
empty to begin with or because it contained only white space),
this calculation will produce a very large `i' that is almost
assuredly wrong, R-O-N-G.
> while(isspace(str[i]))
Bug: Same missing cast as above.
> {
> i--;
> }
> if(i < (strlen(str) - 1))
Bug: Same mishandling of the empty string as above.
Speed: strlen(str) is still the same as it was a few lines
ago, so there's no point in computing it again.
> {
> str[i + 1] = '\0';
Speed: It's probably quicker -- and certainly less verbose --
to do this assignment unconditionally than to test whether it's
needed. If it isn't, you'll just store a zero on top of an
existing zero, which is harmless.
> }
> }
Summary: Not quite ready for prime time, but not as bad as
some attempts I've seen.
Challenge: See if you can think of a way to do the job in
just one pass over the string (calling strlen() counts as a
"pass"). Hint: During the copy-to-front operation, can you
somehow figure out where the final '\0' should land without
going back and inspecting the moved characters a second time?
Thanks for the input. Someone else had pointed out one of the issues
you mentioned but I didn't understsnd what he was saying until I read
your description.
//This was in my original file but just not immediately before this
function
#include <ctype.h>
#include <string.h>
//Added char* return as suggested
char* TrimCString(char *str)
{
// Trim whitespace from beginning:
size_t i = 0;
size_t j;
//Added validation of the argument for NULL
if(str == NULL)
{
return str;
}
//Added cast as suggested. I have always avoided casts for various
reasons
//discussed in the FAQ and elsewhere but it is good to know this
case
while(isspace((unsigned char)str[i]))
{
i++;
}
if(i > 0)
{
//Calculate length once
size_t length = strlen(str);
//Decided to leave the bytewise copy I just think I can
understand it a little better
for(j = 0; i < length; j++, i++)
{
str[j] = str[i];
}
str[j] = '\0';
}
// Trim whitespace from end:
//Added check to catch the empty string
i = (strlen(str) ? (strlen(str) - 1) : 0);
while(isspace((unsigned char)str[i]))
{
i--;
}
//Removed check that was not needed
> > * what if I pass a constant string literal to the function?
> > TrimCString(" 4 spaces at both ends ");
> How can I account for this?
Well ... you can't!
But you can do (at least) one of two things:
a) add a comment forbidding the caller to pass a string literal.
The comment goes near the function in the file with the code,
it also goes in the header file and in the documentation
provided for TrimCString()
b) Rewrite TrimCString to write the trimmed string to a
different memory area (variable, array, whatever), a bit
like strcpy() works
char *TrimCString(char *dest, const char *source)
/* caller must ensure `dest` has enough space for
the trimmed string */
/* if source is NULL, trims `dest` in-place. `dest`
*MUST* be writable */
> swengi...@gmail.com wrote:
>> Just looking for a few eyes on this code other than my own.
<snip>
>> size_t i = 0;
>
> Style: Instead of initializing `i' at its declaration and then
> relying on the initial value later on, consider initializing it
> closer to its use. The `for' statement is convenient for such
> purposes.
If you are suggesting replacing the while (isspace(str[i]) with a
for (int i = 0; isspace((unsigned char)str[i]; i++); then this will not
allow i to tested outside the for:
>> while(isspace(str[i]))
>> {
>> i++;
>> }
>> if(i > 0)
here.
--
Ben.
> //Added char* return as suggested
> char* TrimCString(char *str)
> {
[...]
> //Calculate length once
> size_t length = strlen(str);
> //Decided to leave the bytewise copy [...]
> for(j = 0; i < length; j++, i++)
> {
> str[j] = str[i];
> }
> str[j] = '\0';
> }
>
> // Trim whitespace from end:
> //Added check to catch the empty string
> i = (strlen(str) ? (strlen(str) - 1) : 0);
Here, strlen(str) is the same as j.
You can replace the call to strlen() with j :)
i = j ? j - 1 : 0;
> while(isspace((unsigned char)str[i]))
> {
> i--;
> }
> //Removed check that was not needed
> str[i + 1] = '\0';
And, since you changed the function to return a char *,
do return a char * to the caller.
return str;
> }
<snip>
> // Trim whitespace from end:
> //Added check to catch the empty string
> i = (strlen(str) ? (strlen(str) - 1) : 0);
>
> while(isspace((unsigned char)str[i]))
> {
> i--;
> }
> //Removed check that was not needed
> str[i + 1] = '\0';
> }
2 things I found:
1. You are missing a return statement at the end.
2. This will not be correct if someone calls your function with an array
of length one (with its only element set to '\0'). The index in your
assignment at the last line will then be 1.
--
Michael Brennan
Why move the characters at all? The original version needed
to because it returned no value, so the only way it could report
the position of the first non-space was to squeeze out the leading
spaces. But now that you're returning a pointer, why not just
point at the first non-space no matter where you find it?
> // Trim whitespace from end:
> //Added check to catch the empty string
> i = (strlen(str) ? (strlen(str) - 1) : 0);
>
> while(isspace((unsigned char)str[i]))
> {
> i--;
> }
This still mis-handles the empty string. Work through it:
You'll set `i' to zero, and then you'll test whether `str[0]'
is a space. It's the '\0', so it's not a space, so you decrement
`i' from zero to a very large number. Then you check whether
`str[very_large_number]' is a space, and there's no telling
what will happen except that it's not likely to be good ...
> //Removed check that was not needed
> str[i + 1] = '\0';
> }
You still haven't picked up on the hints about an easier way
to deal with the trailing spaces. Let's try another analogy:
Imagine you're watching a football ("soccer") game that has
reached the shoot-out tie-breaking phase. Abelard shoots and
the goalkeeper makes the save (space). Then Bertrand shoots
and scores (non-space). Then Claude shoots, and Daniel, and
Edouard, ... When the match is over, you know you will be asked
which shooter scored the side's final goal (which may or may not
have been followed by a few saves). Unfortunately, your memory
is terrible -- but you have an erasable slate with enough space
to write one name at a time. Can you think of a method that will
ensure you can answer the question correctly?
missing in my previous post
> > > if (i > 0)
> > > {
> > > //Calculate length once
> > > size_t length = strlen(str);
> > > //Decided to leave the bytewise copy [...]
> > > for(j = 0; i < length; j++, i++)
> > > {
> > > str[j] = str[i];
> > > }
> > > str[j] = '\0';
> > > }
> >
> > > // Trim whitespace from end:
> > > //Added check to catch the empty string
> > > i = (strlen(str) ? (strlen(str) - 1) : 0);
> >
> > Here, strlen(str) is the same as j.
> > You can replace the call to strlen() with j :)
> This is only true if there was whitespace to trim from the front and
> not in the more general case.
Oops ... you're right, thanks for pointing it out.
But I still think you could do without some of those strlen() calls.
I assume he meant C89 style:
for (i = 0; isspace((unsigned char)str[i]; i++);
SaSW, Willem
--
Disclaimer: I am in no way responsible for any of the statements
made in the above text. For all I know I might be
drugged or something..
No I'm not paranoid. You all think I'm paranoid, don't you !
#EOT
Because then the subsequent call to free() would bomb.
#include <ctype.h>
#include <string.h>
char* TrimCString(char *str)
{
// Trim whitespace from beginning:
size_t i = 0;
size_t length = strlen(str);
char* new_start;
if((str == NULL) || (length == 0))
{
return str;
}
while(isspace((unsigned char)str[i]))
{
i++;
}
new_start = str + i;
length -= i;
// Trim whitespace from end:
i = (length ? (length - 1) : length);
if(i != 0)
{
while(isspace((unsigned char)str[i]))
{
i--;
length--;
}
str[i + 1] = '\0';
}
memmove(str, new_start, (length + 1));
return str;
}
Sorry if I wasn't clear: I wanted to suggest moving the
initialization of `i', not its declaration. And the reason
for moving it is that I just hate reading code that goes
int i = 0;
/*
* forty-two lines not mentioning i
*/
while (foo[i++] != bar) ...
In the O.P.'s case the "forty-two" was only two, but fostering
good habits is a Good Thing. Besides, I called it a "style"
point and suggested he "consider" the change; I'm trying to
lead the horse to Kool-Aid[*] without insisting he drink it.
[*] A misattribution, BTW.
Well, the whole thing suffers from the lack of a clear
usage description, as another poster pointed out. But why
should anyone expect the pointer to be free-able? Do you
expect to pass the result of strchr() to free? Or the
end-pointer produced by strtod()?
Unless it's part of the function's contract that it will
return its first argument, there's no reason to want to call
free() on it. And since the O.P. is willing to consider changes
to the interface and there's no contract in evidence, it seems
unwarranted to assume anything special about the returned value.
The thing is still being designed; we can still change the rules.
It probably isn't relevant, but
I profiled 1,000,000 calls of your first and last versions:
index % time self children called name
[2] 94.2 6.26 0.00 1000000 TrimCString1 [2]
[3] 4.7 0.31 0.00 1000000 TrimCString2 [3]
Increase in speed was ( 6.26 / 0.31 = 20+ ) twentyfold :)
You're still working far too hard, IMHO. Ponder this one:
#include <ctype.h>
/* Remove trailing white space from a string, and return
* a pointer to the string's first non-space character
* (which might be its terminating '\0').
*/
char *TrimString(char *str)
{
char *ptr;
char *end;
/* skip leading spaces */
while (isspace( (unsigned char)*str ))
++str;
/* find start of trailing spaces (if any) */
end = str;
for (ptr = str; *ptr != '\0'; ++ptr) {
if (! isspace( (unsigned char)*ptr ))
end = ptr + 1;
}
/* clip off trailing spaces (if any) */
*end = '\0';
return str;
}
Or you might prefer to write the second loop this way:
/* find start of trailing spaces (if any) */
end = ptr = str;
while (*ptr != '\0') {
if (! isspace( (unsigned char)*ptr++ ))
end = ptr;
}
Use whichever seems clearer to you; I think it's a toss-up.
Oh, botheration. Ignore this remark; I'm completely wrong.
Sorry.
Actually, as the functions do not return an int (which under C89 rules
the compiler is required to assume they return) it invokes undefined
behaviour. So although I am not aware of any implementations where it
would fail it still *could* fail.
--
Flash Gordon
> > Just looking for a few eyes on this code other than my own.
>
> > for(j = 0; i < strlen(str); j++, i++)
> > {
> > str[j] = str[i];
> > }
> An alternative would be to use memmove() here, so you don't
> have to do it byte by byte.
What would be the actual metrics to make a decision here
one way or the other? I myself just assume for this kind of stuff
that it will only be done on relatively small strings, just a few
characters, so it seems that a call to memmove() might be overkill.
Or is it? DOES it depend on the size of the block you'll
be moving? How many cycles does it take to assign one
character to a previous array position, versus the overhead of
a call to memmove(), and the operation of the function itself?
> Also callling strlen() each time
> through the loop is a bit of a waste of time - it doesn't
> change and can be replaced by a check if str[i] is '\0'.
Or just do a very simple while() loop that compares pointers:
char *start=str;
char *end=str+strlen(str);
while(start!=end) {
/* do something */
/* start++ for going forward from start */
/* end-- for going backwards from end */
}
Can't go wrong there...I will admit that I use to always use array
sub-scripting such as the OP because I found it easier conceptually
to understand, but then abandoned it largely for the equivalent
pointers as part of my paranoia about compiler "optimizations"
(I turn them ALL off, which means sub-scripting is not converted
to pointers, which I believe takes longer, so to retain the speed
I've manually converted most of the stuff like this).
---
William Ernest Reid
> On Jul 10, 1:32 pm, j...@toerring.de (Jens Thoms Toerring) wrote:
> > swengineer...@gmail.com <swengineer...@gmail.com> wrote:
[snip]
> > > size_t i = 0;
[snip]
> > > // Trim whitespace from end:
> > > i = strlen(str) - 1;
> >
> > Careful: This could set 'i' to -1 (if the string consistet of white
> > space only) and then the rest won't work anymore.
> i is of type size_t which I believe can't be negative?
You are correct that size_t is an unsigned integer type, and can never
be negative. But that doesn't mean that underflowing it won't cause a
very serious problem. Consider:
#include <stdio.h>
int main(void)
{
size_t i = 0;
--i;
printf("%lu\n", (unsigned long)i);
return 0;
}
Compile and execute this and see what you get.
--
Jack Klein
Home: http://JK-Technology.Com
FAQs for
comp.lang.c http://c-faq.com/
comp.lang.c++ http://www.parashift.com/c++-faq-lite/
alt.comp.lang.learn.c-c++
http://www.club.cc.cmu.edu/~ajo/docs/FAQ-acllc.html
#include <stdio.h>
#include <string.h>
#include <ctype.h>
#include <stdlib.h>
char *stringtrim(char *str)
{
char *res, *ini, *end, *ptr;
int size;
/* safe mode to NULL string */
if (!str)
return NULL;
size = strlen(str);
if (!size)
return NULL;
ini = str;
/* go to the first non blank character */
while (isspace((unsigned char)*ini)) {
++ini;
--size;
}
if (size <= 0)
return NULL;
end = str + strlen(str) - 1; /* point to the last character */
/* go to the last non blank character */
while (isspace((unsigned char)*end)) {
--end;
--size;
}
if (size <= 0)
return NULL;
res = malloc(sizeof(char) * size);
for (ptr = res; ini != (end+1); ini++)
*ptr++ = *ini;
*ptr = '\0';
return res;
}
int main(void)
{
printf("|%s|\n", stringtrim(" hello, world"));
printf("|%s|\n", stringtrim(" 1234 5678 90 "));
char teste[] = " good bye, cruel world ";
printf("|%s|\n", stringtrim(teste));
return 0;
}
As far as I know, there is no speed difference between sub-scripting
and pointers on most modern CPUs.
I think this is more of a stylistic difference than anything else. For
me I feel my version is more understandable. For you maybe not.
I am curious though, did I address your concerns about an empty string
sufficiently?
Thanks for the help.
Interesting. I would not have guessed it would be that much of a
difference.
I did not introduce any bugs with this change that you see did I? In
my testing it still performs as I would expect.
Why stringtrim("") returns NULL?
"" is a perfectly valid string.
> ini = str;
>
> /* go to the first non blank character */
> while (isspace((unsigned char)*ini)) {
> ++ini;
> --size;
> }
>
> if (size <= 0)
> return NULL;
Why stringtrim(" ") returns NULL?
"" is a perfectly valid string.
> end = str + strlen(str) - 1; /* point to the last character */
> /* go to the last non blank character */
> while (isspace((unsigned char)*end)) {
> --end;
> --size;
> }
>
> if (size <= 0)
> return NULL;
>
> res = malloc(sizeof(char) * size);
> for (ptr = res; ini != (end+1); ini++)
> *ptr++ = *ini;
> *ptr = '\0';
>
> return res;
> }
There's a memory leak in your program.
You never free the memory you malloc'd for the result.
[snip]
Oh ... `stringtrim` is a reserved name
http://www.gnu.org/software/libtool/manual/libc/Reserved-Names.html
Actually, now that I look at this a little closer won't this truncate
a string with a space in the middle. That is not what I want. A space
in the middle is fine I just want to trim off the front and back.
> On Jul 10, 5:01 pm, Eric Sosman <Eric.Sos...@sun.com> wrote:
<snip>
>> /* find start of trailing spaces (if any) */
>> end = str;
>> for (ptr = str; *ptr != '\0'; ++ptr) {
>> if (! isspace( (unsigned char)*ptr ))
>> end = ptr + 1;
>> }
>>
>> /* clip off trailing spaces (if any) */
>> *end = '\0';
>>
>> return str;
>> }
>>
>> Or you might prefer to write the second loop this way:
>>
>> /* find start of trailing spaces (if any) */
>> end = ptr = str;
>> while (*ptr != '\0') {
>> if (! isspace( (unsigned char)*ptr++ ))
>> end = ptr;
>> }
>>
>> Use whichever seems clearer to you; I think it's a toss-up.
>>
>> --
>> Eric.Sos...@sun.com- Hide quoted text -
>>
>> - Show quoted text -
>
> Actually, now that I look at this a little closer won't this truncate
> a string with a space in the middle.
Not as far as I can see. In both cases end is set to point just after
something that is know not to be space. Because the loop runs to the
end of the string, end points just after the *last* thing know not to
be a space. This will be the null or the first trailing space.
--
Ben.
Hmmm ... if I pass NULL to your TrimCString it'll try to do strlen on
that.
I'm not sure you can do that.
: char* TrimCString(char *str)
: {
: // Trim whitespace from beginning:
: size_t i = 0;
: size_t length = strlen(str); /* <=== strlen(NULL)
is ok? */
: char* new_start;
:
: if((str == NULL) || (length == 0))
: {
: return str;
: }
: [rest of program]
Maybe try this instead
char* TrimCString(char *str)
{
// Trim whitespace from beginning:
size_t i = 0;
size_t length; /* not initialized */
char* new_start;
if(str == NULL) /* only test for NULL */
{
return str;
}
length = strlen(str);
if(length == 0) /* only test for
length == 0 */
{
return str;
}
[rest of program]
All this is system-specific.
[snip]
> Can't go wrong there...I will admit that I use to always use array
> sub-scripting such as the OP because I found it easier conceptually
> to understand, but then abandoned it largely for the equivalent
> pointers as part of my paranoia about compiler "optimizations"
> (I turn them ALL off, which means sub-scripting is not converted
> to pointers, which I believe takes longer, so to retain the speed
> I've manually converted most of the stuff like this).
Subscripting might be faster than pointer manipulation on some
systems; pointer manipulation might be faster on others. A decent
optimizing compiler will know which is faster, and can often convert
either form into whichever is best for the target system.
Consider letting the compiler do its job.
--
Keith Thompson (The_Other_Keith) ks...@mib.org <http://www.ghoti.net/~kst>
Nokia
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
> ) (I turn them ALL off, which means sub-scripting is not converted
> ) to pointers, which I believe takes longer, so to retain the speed
> ) I've manually converted most of the stuff like this).
>
> As far as I know, there is no speed difference between sub-scripting
> and pointers on most modern CPUs.
DAMN!!! More time I've wasted because I JUST WON'T LISTEN!!!
In any event, does the following win the prize for most efficient
implementation of the presumed requirements of the function?
char *remove_beg_end_non_text(char *text) {
char *beg;
size_t length;
for(beg=text;*beg!='\0';beg++)
if(!isspace(*beg)) break;
length=strlen(beg);
while(length>0)
if(!isspace(*(beg+(--length)))) break;
*(beg+(++length))='\0';
return beg==text ? text : memmove(text,beg,length+1);
}
Gotta admit, you couldn't reduce the cycles too much on
that, could you? And it could even win a little bonus prize
for obfuscatory conditions like if(!isspace(*(beg+(--length))))...
---
William Ernest Reid
This is not really responsive. I make a distinction between doing
an "delete" of about 20 characters from a text block of 300K and moving
10 characters one position forward. I find it difficult to believe that
some "specific" systems make no or counter-intuitive operational
distinctions between the two different methods of accomplishing the
operation.
But I'm "listening"; tell me how many cycles different "systems"
would consume to perform each method...or is memmove() kind
of like fread() and fwrite(), you THINK that they're doing something
"special", but really they're using fgetc() and fputc() which in most
cases you probably could have just used yourself and eliminated
the "middleman"...
> [snip]
>
> > Can't go wrong there...I will admit that I use to always use array
> > sub-scripting such as the OP because I found it easier conceptually
> > to understand, but then abandoned it largely for the equivalent
> > pointers as part of my paranoia about compiler "optimizations"
> > (I turn them ALL off, which means sub-scripting is not converted
> > to pointers, which I believe takes longer, so to retain the speed
> > I've manually converted most of the stuff like this).
>
> Subscripting might be faster than pointer manipulation on some
> systems; pointer manipulation might be faster on others. A decent
> optimizing compiler will know which is faster, and can often convert
> either form into whichever is best for the target system.
>
> Consider letting the compiler do its job.
The compiler has "told" me which it considers faster, since it is
documented that the default optimization is to replace sub-scripts
with pointers. Therefore I cannot really be preventing the compiler
from doing its "job" (your opinion) by pre-emptively writing my
code to do the same thing as the "optimizer", at least for this
particular "optimization".
Riiiiiiiiiiiiiiight? Now as far as some other optimizations are
concerned, I can't re-write my code to emulate those, but THOSE
tend to be the ones I am really concerned about in the first place,
with GOOD reason...
---
William Ernest Reid
No :)
> char *remove_beg_end_non_text(char *text) {
> char *beg;
> size_t length;
>
> for(beg=text;*beg!='\0';beg++)
> if(!isspace(*beg)) break;
BUG: missing cast
> length=strlen(beg);
>
> while(length>0)
> if(!isspace(*(beg+(--length)))) break;
BUG: missing cast
> *(beg+(++length))='\0';
>
> return beg==text ? text : memmove(text,beg,length+1);
> }
Also
remove_beg_end_non_text(NULL);
crashes on my computer.
> And it could even win a little bonus prize
> for obfuscatory conditions like if(!isspace(*(beg+(--length))))...
Obfuscatory conditions never win prizes (except at IOCCC).
I think you have a bug.
> char *remove_beg_end_non_text(char *text) {
> char *beg;
> size_t length;
>
> for(beg=text;*beg!='\0';beg++)
> if(!isspace(*beg)) break;
I'd guess
for (beg = text; isspace((unsigned char)*beg); beg++);
makes shorter code with some compilers.
> length=strlen(beg);
>
> while(length>0)
> if(!isspace(*(beg+(--length)))) break;
>
> *(beg+(++length))='\0';
If length never is never decremented (because it was zero after the
initial space scan) then this writes outside the string. It always
helps to walk through what your code does in boundary cases like ""
and " ".
> return beg==text ? text : memmove(text,beg,length+1);
> }
>
> Gotta admit, you couldn't reduce the cycles too much on
> that, could you? And it could even win a little bonus prize
> for obfuscatory conditions like if(!isspace(*(beg+(--length))))...
You might have obscure it even to yourself!
--
Ben.
You misunderstand. Keith's point is that how this is done is not defined
by hte C language, it is specific to individual implementations. Your
system may do it differently to mine.
So to get a good answer, you need to ask in a system-specific news group.
> But I'm "listening"; tell me how many cycles different "systems"
> would consume to perform each method...
Between 0 and a billion, depending on how your particular system
implements the function.....
--
Mark McIntyre
CLC FAQ <http://c-faq.com/>
CLC readme: <http://www.ungerhu.com/jxh/clc.welcome.txt>
Valid complaint: Although most of your paragraph was about
system-specific nonsense, the actual question "What would be
the actual metrics?" can be addressed at least in part without
system specificity. Here are some "actual metrics" for your
consideration; others may occur to your thought:
- Brevity
- Clarity
- Maintainability
- Program size
- Execution time
- Price of tea in China
> But I'm "listening"; tell me how many cycles different "systems"
> would consume to perform each method...or is memmove() kind
> of like fread() and fwrite(), you THINK that they're doing something
> "special", but really they're using fgetc() and fputc() which in most
> cases you probably could have just used yourself and eliminated
> the "middleman"...
All this is system-specific.
--
Eric Sosman
eso...@ieee-dot-org.invalid
> <snip>
> > In any event, does the following win the prize for most efficient
> > implementation of the presumed requirements of the function?
>
> I think you have a bug.
>
> > char *remove_beg_end_non_text(char *text) {
> > char *beg;
> > size_t length;
> >
> > for(beg=text;*beg!='\0';beg++)
> > if(!isspace(*beg)) break;
>
> I'd guess
>
> for (beg = text; isspace((unsigned char)*beg); beg++);
>
> makes shorter code with some compilers.
Yeah, but does it fly past the terminating null character? I think
you may have out-obfuscated me...
Also, I've not been following why the cast is important here...
> > length=strlen(beg);
> >
> > while(length>0)
> > if(!isspace(*(beg+(--length)))) break;
> >
> > *(beg+(++length))='\0';
>
> If length never is never decremented (because it was zero after the
> initial space scan) then this writes outside the string. It always
> helps to walk through what your code does in boundary cases like ""
> and " ".
Actually, the "boundary case" for this is the size of array; as long
as it is at least one character bigger than the "string" then this will
always work. But you are correct if you replace the word "string" with
the word "array fully-populated by the string" above.
There are actually only five possible test cases, which all should
ASSUME a "array fully-populated by the string" (which I unfortunately
didn't): spaces before text, spaces after the text, spaces before and
after the text, just spaces, empty string (six if you count the stupidity
of passing NULL). Pass those five (or six) cases, the thing is
perfect...
> > return beg==text ? text : memmove(text,beg,length+1);
> > }
> >
> > Gotta admit, you couldn't reduce the cycles too much on
> > that, could you? And it could even win a little bonus prize
> > for obfuscatory conditions like if(!isspace(*(beg+(--length))))...
>
> You might have obscure it even to yourself!
OK, I need to work on a few things to make it the perfect piece
of obscure ruthlessly efficient code...
---
William Ernest Reid
> > In any event, does the following win the prize for most efficient
> > implementation of the presumed requirements of the function?
>
> No :)
Honorable mention?
> > char *remove_beg_end_non_text(char *text) {
> > char *beg;
> > size_t length;
> >
> > for(beg=text;*beg!='\0';beg++)
> > if(!isspace(*beg)) break;
>
> BUG: missing cast
I've not been following why the cast is important; is this some type
of "error" that has never actually occured on the planet Earth but MIGHT
happen in the unpredictable future?
> > length=strlen(beg);
> >
> > while(length>0)
> > if(!isspace(*(beg+(--length)))) break;
>
> BUG: missing cast
IBID.
> > *(beg+(++length))='\0';
> >
> > return beg==text ? text : memmove(text,beg,length+1);
> > }
You missed a bug!
> Also
> remove_beg_end_non_text(NULL);
> crashes on my computer.
Of course it does, so why did you do it? Just how far down do
we have to push error checking? If your answer is "to the depths
of hell", this will make you happy, but I'll bet you'll still manage
to crash your computer SOMEHOW if you do stuff like that:
if(text==NULL) return text;
> > And it could even win a little bonus prize
> > for obfuscatory conditions like if(!isspace(*(beg+(--length))))...
>
> Obfuscatory conditions never win prizes (except at IOCCC).
Even if they actually make things work "better"?
---
William Ernest Reid
You misunderstand the English language, since I specifically used
the word "system" above.
> So to get a good answer, you need to ask in a system-specific news group.
OK, I'll take that as a "I have no idea"...
> > But I'm "listening"; tell me how many cycles different "systems"
> > would consume to perform each method...
>
> Between 0 and a billion, depending on how your particular system
> implements the function.....
OK, I'll take that as a "I REALLY have no idea", since all you had
to do was pick ANY system (or two or three) that you were familiar with
to answer the question...
---
William Ernest Reid
I am left wondering how you manage to cope with everyday life
with such poor reading comprehension. I specifically asked about
"cycles", and you respond with "price of tea"...
> > But I'm "listening"; tell me how many cycles different "systems"
> > would consume to perform each method...or is memmove() kind
> > of like fread() and fwrite(), you THINK that they're doing something
> > "special", but really they're using fgetc() and fputc() which in most
> > cases you probably could have just used yourself and eliminated
> > the "middleman"...
>
> All this is system-specific.
I was looking for the "Answers Department", but accidentally walked
into the "Department of Begging the Question" (part of the "Clueless
Pedants Department", sharing a common desk with the "Pointless
Arguments Department")...
In any event, I guess "Jens Thoms Toerring" was totally off-base in
suggesting memmove() over re-writing the string in place, since he
clearly didn't have access to the non-information about the specific
"system"...
---
William Ernest Reid
Then you need to practice believing more things. :-)
On some systems, a call to memmove() will generate an actual call to a
library function that moves one byte at a time just like your loop, and
thus will always be slower.
On other systems, a call to memmove() will generate an actual call to a
library function that uses a few (maybe even just one) machine
instructions to move the bytes much faster than your byte at a time
loop. Which is faster depends on the number of bytes moved, how much
overhead there is in a function call, and how much faster the machine
instructions are than your loop. It is likely that your loop will be
faster for "short" strings and slower for "long" strings, but exactly
where the break-even point is will vary from system to system. It might
be just a few bytes, it might be tens of bytes, it might even be
hundreds of bytes (although probably not).
On still other systems, a call to memmove() will generate those few (or
maybe even just one) machine instructions inline, which will almost
certainly be faster than your byte at a time loop all the time.
> or is memmove() kind
> of like fread() and fwrite(), you THINK that they're doing something
> "special", but really they're using fgetc() and fputc() which in most
> cases you probably could have just used yourself and eliminated
> the "middleman"...
Although the C standard describes fread()/fwrite() as working as if they
called fgetc()/fputc() for each byte, I've never seen an implementation
where they actually do that; real implementations nearly always *do* do
something special instead. On the other hand, I've seen actual
implementations use all three of the above methods of implementing
memmove().
--
Larry Jones
All this was funny until she did the same thing to me. -- Calvin
Ok, here's a more responsive answer.
I don't know.
Specifically, I don't know how many cycles any given operation will
take on whatever system you're using, partly because I don't know what
system you're using. Also, I don't know how many cycles it would take
on the system(s) *I'm* using, since I've never bothered to measure it.
I would expect that a call to memmove() would be faster than an
equivalent explicit loop on some systems, and vice versa on others.
memmove() has some overhead to determine whether the operands overlap,
but if the compiler can determine that they don't, or if it knows
which way they overlap and knows how memcpy() behaves, it might be
able to avoid that overhead. memmove() or memcpy() likely has some
overhead due to the function call, but an optimizing compiler might
replace either with inline code in some cases. An implementation of
memmove() or memcpy() might take advantage of copying chunks larger
than 1 byte; whether it can do this might depend on the alignment of
the operands. And so forth.
Perfect knowledge of the C language (something I don't claim to have)
would not enable someone to answer your question; that should be a
clue that this is not the place to ask it. Consider measuring it
yourself, on your own system with your own code.
[...]
> But I'm "listening";
I won't speculate on what the quotation marks are supposed to mean.
> tell me how many cycles different "systems"
> would consume to perform each method...
See above.
> or is memmove() kind
> of like fread() and fwrite(), you THINK that they're doing something
> "special", but really they're using fgetc() and fputc() which in most
> cases you probably could have just used yourself and eliminated
> the "middleman"...
The behavior of fread() and fwrite() is defined in terms of fgetc()
and fputc(), but there's no requirement that they actually be
implemented that way. Conversely, fgetc() and fputc() typically use
buffering, which means that 1000 calls to fputc() won't necessarily be
faster or slower than an equivalent single call to fwrite(). Use
whatever is clearer.
[...]
>> Consider letting the compiler do its job.
>
> The compiler has "told" me which it considers faster, since it is
> documented that the default optimization is to replace sub-scripts
> with pointers. Therefore I cannot really be preventing the compiler
> from doing its "job" (your opinion) by pre-emptively writing my
> code to do the same thing as the "optimizer", at least for this
> particular "optimization".
Perhaps for this particular compiler. Other compilers for other
systems might behave differently.
> Riiiiiiiiiiiiiiight? Now as far as some other optimizations are
> concerned, I can't re-write my code to emulate those, but THOSE
> tend to be the ones I am really concerned about in the first place,
> with GOOD reason...
--
> Ben Bacarisse <ben.u...@bsb.me.uk> wrote in message
> news:87iqvbj...@bsb.me.uk...
>> "Bill Reid" <horme...@happyhealthy.net> writes:
>
>> <snip>
>> > In any event, does the following win the prize for most efficient
>> > implementation of the presumed requirements of the function?
>>
>> I think you have a bug.
>>
>> > char *remove_beg_end_non_text(char *text) {
>> > char *beg;
>> > size_t length;
>> >
>> > for(beg=text;*beg!='\0';beg++)
>> > if(!isspace(*beg)) break;
>>
>> I'd guess
>>
>> for (beg = text; isspace((unsigned char)*beg); beg++);
>>
>> makes shorter code with some compilers.
>
> Yeah, but does it fly past the terminating null character?
Nope. Why would you think that?
> I think
> you may have out-obfuscated me...
I am happy to oblige. Those were your rules: you wanted minimal code;
your own example suggests clarity does not matter (although I think my
loop is clearer, but that is simply option).
> Also, I've not been following why the cast is important here...
>
>> > length=strlen(beg);
>> >
>> > while(length>0)
>> > if(!isspace(*(beg+(--length)))) break;
>> >
>> > *(beg+(++length))='\0';
>>
>> If length never is never decremented (because it was zero after the
>> initial space scan) then this writes outside the string. It always
>> helps to walk through what your code does in boundary cases like ""
>> and " ".
>
> Actually, the "boundary case" for this is the size of array; as long
> as it is at least one character bigger than the "string" then this will
> always work. But you are correct if you replace the word "string" with
> the word "array fully-populated by the string" above.
>
> There are actually only five possible test cases, which all should
> ASSUME a "array fully-populated by the string" (which I unfortunately
> didn't): spaces before text, spaces after the text, spaces before and
> after the text, just spaces, empty string (six if you count the stupidity
> of passing NULL). Pass those five (or six) cases, the thing is
> perfect...
I am getting lost in all the words. I think your code is wrong (in
two of the five cases you are considering). Are you saying it is
correct? Like many "off by one" bugs it may not produce undefined
behaviour. For example passing char test[2] = ""; is fine because of
the extra byte, but it is still a bug.
>> > return beg==text ? text : memmove(text,beg,length+1);
>> > }
>> >
>> > Gotta admit, you couldn't reduce the cycles too much on
>> > that, could you? And it could even win a little bonus prize
>> > for obfuscatory conditions like if(!isspace(*(beg+(--length))))...
>>
>> You might have obscure it even to yourself!
>
> OK, I need to work on a few things to make it the perfect piece
> of obscure ruthlessly efficient code...
I'd want to be sure it is correct first. I would be the first admit I
make mistakes but you have not persuaded me that I am wrong about
this. I tried to be as clear about the bug as possible.
--
Ben.
Answers for what sort of questions? The price of tea in china? how to
iron shirts? Did you try yahoo answers?
but accidentally walked
> into the "Department of Begging the Question" (part of the "Clueless
> Pedants Department", sharing a common desk with the "Pointless
> Arguments Department")...
Feel free to insult the experts who hang out here, I'm sure they'll be
even keener to help you after you've been rude to them.
> ---
> William Ernest Reid
ps one too many dashes, and no space in your sigsep.
Take it how you like. It means what it says.
For the record, I have several answers, depending on which system you're
interested in. However you have zero chance of getting any of them from
me now.
>all you had
> to do was pick ANY system (or two or three) that you were familiar with
> to answer the question...
And had you asked the question over in a system-specific group, I'd have
answered with a relevant answer.
However its not topical here, so given your abusive attitude I suggest
you swivel on it.
(spaces added for legibility above)
>>
>> BUG: missing cast
>
> I've not been following why the cast is important; is this some
> type of "error" that has never actually occured on the planet
> Earth but MIGHT happen in the unpredictable future?
Serious bug. isspace(int) requires the integer value of an
unsigned char. Repeat, unsigned. If the char type on any machine
is signed isspace can receive a negative value, and quite likely
blowup. That's why the argument to isspace should receive an
explicit cast. The only negative value those functions can receive
is EOF.
The cast can be avoided when the integer output of getc() is
passed, because getc returns the appropriately cast value in the
first place. But you are getting those chars from a string, and
whether a raw char is signed or unsigned is always undefined.
--
[mail]: Chuck F (cbfalconer at maineline dot net)
[page]: <http://cbfalconer.home.att.net>
Try the download section.
<snip>
> The cast can be avoided when the integer output of getc() is
> passed, because getc returns the appropriately cast value in the
> first place. But you are getting those chars from a string, and
> whether a raw char is signed or unsigned is always undefined.
Your major point is correct, but you have one detail wrong. Whether a raw
char is signed or unsigned is never undefined - it is always
implementation-defined. "If a member of the required source character set
enumerated in $2.2.1 is stored in a char object, its value is guaranteed
to be positive. If other quantities are stored in a char object, the
behavior is implementation-defined: the values are treated as either
signed or nonnegative integers."
--
Richard Heathfield <http://www.cpax.org.uk>
Email: -http://www. +rjh@
Google users: <http://www.cpax.org.uk/prg/writings/googly.php>
"Usenet is a strange place" - dmr 29 July 1999
Repetition is not clarification. By declaration, and all available
"official" documentation, isspace(int) requires a signed integer value...
> If the char type on any machine
> is signed isspace can receive a negative value, and quite likely
> blowup.
On my machine, "char" is "signed char" by default, at least
according to SOME of the documentation. Nothing is "blowing
up". Why so?
> That's why the argument to isspace should receive an
> explicit cast. The only negative value those functions can receive
> is EOF.
Sure you're not conflating "strings" with "streams"?
> The cast can be avoided when the integer output of getc() is
> passed, because getc returns the appropriately cast value in the
> first place.
How do you think the characters wound up in a string in the first
place?
> But you are getting those chars from a string, and
> whether a raw char is signed or unsigned is always undefined.
I appear to "getting" a signed 8-bit integer value from my string,
with no problems, yet you think it is critically important to cast
it to an unsigned integer value, for a function that takes a signed
integer value as an argument, in order to perform a look-up of
certain integer values. Nope, still not getting it...
---
William Ernest Reid
Oh, you know, the fact that you aren't actually testing for
the null character, you're just relying on isspace() to return
0 for it, which probably is correct in all cases, but just
"looks dangerous"...
No, it is not correct. You're right, it is a TRUE "bug".
> Like many "off by one" bugs it may not produce undefined
> behaviour. For example passing char test[2] = ""; is fine because of
> the extra byte, but it is still a bug.
Of course. The way I allocate arrays for text on THIS system
means you might NEVER encounter it, but it is still wrong...
> >> > return beg==text ? text : memmove(text,beg,length+1);
> >> > }
> >> >
> >> > Gotta admit, you couldn't reduce the cycles too much on
> >> > that, could you? And it could even win a little bonus prize
> >> > for obfuscatory conditions like if(!isspace(*(beg+(--length))))...
> >>
> >> You might have obscure it even to yourself!
> >
> > OK, I need to work on a few things to make it the perfect piece
> > of obscure ruthlessly efficient code...
>
> I'd want to be sure it is correct first. I would be the first admit I
> make mistakes but you have not persuaded me that I am wrong about
> this. I tried to be as clear about the bug as possible.
Don't worry about it, you're right. Also, I'm giving up on writing
efficient obcure code for this. Aside from the cast "issues", here's
something that is totally "straight-forward" but probably not the
most "efficient" algorithm for all cases, or even any (it is also what
I actually use for small strings):
char *remove_beg_end_non_text(char *text) {
char *curr_char,*text_char=text;
size_t spaces=0;
for(curr_char=text;*curr_char!='\0';curr_char++)
if(!isspace(*curr_char)) break;
while(*curr_char!='\0') {
if(isspace(*curr_char)) spaces++;
else spaces=0;
*text_char++=*curr_char++;
}
*(text_char-spaces)='\0';
return text;
}
ASIDE FROM THE CAST "ISSUES", where's the bug in THAT?
---
William Ernest Reid
That's what Chuck is saying too. But you are passing it a plain char
value without a cast to unsigned. If plain char happens to be unsigned
on your implementation then things are fine, but is it a risk that you
want to take?
[ ... ]
>> That's why the argument to isspace should receive an
>> explicit cast. The only negative value those functions can receive
>> is EOF.
>
> Sure you're not conflating "strings" with "streams"?
How so? As far as I can see he is talking about neither, but the
interface of the is* and to* functions, which can be used with both
strings and text streams.
<snip>
"If it works, it's not sufficiently tested." I'll bet you
a cookie that either (1) char is in fact not signed on your
systems, or (2) you haven't tested "Götterdămmerung" or "Aďda"
or "µsec" or "garçon" or "Ł1 is worth Ą210" or ...
>> The cast can be avoided when the integer output of getc() is
>> passed, because getc returns the appropriately cast value in the
>> first place.
>
> How do you think the characters wound up in a string in the first
> place?
I guess your implication is that they were converted from
int values returned by getc(). Note that "converted from" is
not "copied from."
> I appear to "getting" a signed 8-bit integer value from my string,
> with no problems, yet you think it is critically important to cast
> it to an unsigned integer value, for a function that takes a signed
> integer value as an argument, in order to perform a look-up of
> certain integer values. Nope, still not getting it...
Evidently. Let's try it in small, simple steps:
1) The <ctype.h> functions take an argument of type int.
2) The value of the int argument must be in the range zero
through UCHAR_MAX (all positive) or else EOF (negative).
3) Passing a positive argument >UCHAR_MAX or a negative
argument !=EOF invokes undefined behavior.
With me so far? Okay, stick with it:
4) On an implementation where char is signed, some char
values are negative. (Since the "basic execution set"
characters are all non-negative, lackadaisacal testing
will not reveal this.)
5) At least 126 of those negative values are !=EOF.
6) Therefore, passing one of those char values to a <ctype.h>
function invokes undefined behavior.
--
Eric Sosman
eso...@ieee-dot-org.invalid
char*
trim( char* str )
{
char
* cpy = str,
* seq = str,
* fin = str + strlen( str ) - 1;
while( seq <= fin && isspace( *seq ) )
++seq;
while( seq <= fin && isspace( *fin ) )
--fin;
while( seq <= fin )
*cpy++ = *seq++;
*cpy = 0;
return str;
}
I can't guarantee that it's bug free of course...
>> Therefore, passing one of those char values to a <ctype.h> function invokes undefined behavior
Well, insofar as strictly using isspace as an iterative condition,
yes. Otherwise, it really doesn't matter, does it?
How many negative values are there in those characters that
are not part of the ASCII character set? Does the concept of
"sign-preserving" come into play here?
As far as whether my "system" uses a default signed char,
in one place in the documentation they state that they USED
to do it that way, IMPLYING they don't any more, but don't worry
about it in any event, then in another place they EXPLICITLY
state that's how they do it currently.
Bastards...
> >> The cast can be avoided when the integer output of getc() is
> >> passed, because getc returns the appropriately cast value in the
> >> first place.
> >
> > How do you think the characters wound up in a string in the first
> > place?
>
> I guess your implication is that they were converted from
> int values returned by getc(). Note that "converted from" is
> not "copied from."
There's not that many place where I personally get characters,
and explictly avoid foreign currency symbols (not necessarily the
currencies themselves, especially considering the plummeting
value of the dollar, but I there's just a lot of stuff that never makes
it into my system), and if it did, I'd first think about changing
my "locale", except that would probably be a waste of time (see
end of post)...
> > I appear to "getting" a signed 8-bit integer value from my string,
> > with no problems, yet you think it is critically important to cast
> > it to an unsigned integer value, for a function that takes a signed
> > integer value as an argument, in order to perform a look-up of
> > certain integer values. Nope, still not getting it...
>
> Evidently. Let's try it in small, simple steps:
>
> 1) The <ctype.h> functions take an argument of type int.
>
> 2) The value of the int argument must be in the range zero
> through UCHAR_MAX (all positive) or else EOF (negative).
WHO TOL YA DAT!!! WHO TOL YA DAT!!! Although it kind of
makes sense, since by definition a "C" type would be in that range...
> 3) Passing a positive argument >UCHAR_MAX or a negative
> argument !=EOF invokes undefined behavior.
Well, maybe... (now THAT'S "undefined behavior"!!!).
> With me so far? Okay, stick with it:
>
> 4) On an implementation where char is signed, some char
> values are negative. (Since the "basic execution set"
> characters are all non-negative, lackadaisacal testing
> will not reveal this.)
OK, where does "locale" play into this?
> 5) At least 126 of those negative values are !=EOF.
OK, where does "locale" play into this?
> 6) Therefore, passing one of those char values to a <ctype.h>
> function invokes undefined behavior.
Which of course, includes exactly the expected behavior as one
of the possibilities...
Enter string with air
-> Götterdammerung
original string (19 characters):
Götterdammerung |
new string (15 characters):
Götterdammerung|
(I put that little bar "|" at the end of my printf()s showing the before
and after strings so I could see where the string ends, as well as
the string length.) Well, that one worked...let's keep going, OKAY??!!?!!
ARE YOU WITH ME SO FAR??!??!!
Enter string with air
-> Aïda
original string (8 characters):
Aïda|
new string (4 characters):
Aïda|
I'M LOOKING FORWARD TO THAT COOKIE!!!
Enter string with air
-> µsec
original string (4 characters):
µsec|
new string (4 characters):
µsec|
Enter string with air
-> garçon
original string (24 characters):
garçon |
new string (6 characters):
garçon|
Enter string with air
-> £1 is worth ¥210
original string (24 characters):
£1 is worth ¥210 |
new string (16 characters):
£1 is worth ¥210|
I WANT MY COOKIE!!!!!!
ANY OTHER OF YOU IGNORANT BASTARDS WANT TO GIVE
ME SOME FREE FOOD???!!??!!
---
William Ernest Reid
Let's see. Missing <string.h>, missing <ctype.h>,
potential undefined behavior if the argument points to the
'\0' of an empty string, potential undefined behavior from
misuse of isspace() -- other than that, Mrs. Lincoln, what
did you think of the play?
--
Eric Sosman
eso...@ieee-dot-org.invalid
> Eric Sosman <eso...@ieee-dot-org.invalid> wrote in message
> news:sL-dnVXFp-IO3OfV...@comcast.com...
>> Bill Reid wrote:
<snip>
<snip examples of program working>
> I WANT MY COOKIE!!!!!!
>
> ANY OTHER OF YOU IGNORANT BASTARDS WANT TO GIVE
> ME SOME FREE FOOD???!!??!!
Eric Sosman is about as far from ignorant (on things topical here) as
I can imagine. You asked about an issue. He explained the problem.
You showed that some programs do what you expect on your system, which
in no way invalidates the explanation of the problem. OK,
misunderstandings happen, but then you then go on to be rude to the
person trying to explain this to you.
Why would anyone want to try again?
--
Ben.
If char is signed, 127 or more.
> Does the concept of
> "sign-preserving" come into play here?
Hunnh?
> As far as whether my "system" uses a default signed char,
> in one place in the documentation they state that they USED
> to do it that way, IMPLYING they don't any more, but don't worry
> about it in any event, then in another place they EXPLICITLY
> state that's how they do it currently.
>
> Bastards...
There are at least two easy ways to find out by experiment.
>> 1) The <ctype.h> functions take an argument of type int.
>>
>> 2) The value of the int argument must be in the range zero
>> through UCHAR_MAX (all positive) or else EOF (negative).
>
> WHO TOL YA DAT!!! WHO TOL YA DAT!!!
A document that seems unfamiliar to you, but that has been
adopted by various international and national standards bodies.
>> 4) On an implementation where char is signed, some char
>> values are negative. (Since the "basic execution set"
>> characters are all non-negative, lackadaisacal testing
>> will not reveal this.)
>
> OK, where does "locale" play into this?
Sitting in with Basie and the boys? The locale setting may
affect the result isspace() returns for a given argument, but
does not change set of valid arguments.
>> 5) At least 126 of those negative values are !=EOF.
>
> OK, where does "locale" play into this?
Tenor sax backing up Billie Holliday.
>> 6) Therefore, passing one of those char values to a <ctype.h>
>> function invokes undefined behavior.
>
> Which of course, includes exactly the expected behavior as one
> of the possibilities...
Right. Or, "Hundreds of times don't nuthin' happen a-tall,"
says an old joke about the unimportance of contraception.
> [...]
> I WANT MY COOKIE!!!!!!
>
> ANY OTHER OF YOU IGNORANT BASTARDS WANT TO GIVE
> ME SOME FREE FOOD???!!??!!
You haven't yet shown that char is in fact signed on your
system. In a way, it is both shameful and laudable that you
don't know ...
--
Eric Sosman
eso...@ieee-dot-org.invalid
>
>Eric Sosman <eso...@ieee-dot-org.invalid> wrote in message
>news:sL-dnVXFp-IO3OfV...@comcast.com...
>> Bill Reid wrote:
>> > CBFalconer <cbfal...@yahoo.com> wrote in message
>> >>
>> >> If the char type on any machine
>> >> is signed isspace can receive a negative value, and quite likely
>> >> blowup.
>> >
>> > On my machine, "char" is "signed char" by default, at least
>> > according to SOME of the documentation. Nothing is "blowing
>> > up". Why so?
>>
>> "If it works, it's not sufficiently tested." I'll bet you
>> a cookie that either (1) char is in fact not signed on your
>> systems, or (2) you haven't tested "Götterdămmerung" or "Aďda"
>> or "µsec" or "garçon" or "Ł1 is worth Ą210" or ...
It is in the Standard, in the very first paragraph of the section of
that discusses the ctype.h functions. You know, the document you
don't want to read.
>
>> 3) Passing a positive argument >UCHAR_MAX or a negative
>> argument !=EOF invokes undefined behavior.
>
>Well, maybe... (now THAT'S "undefined behavior"!!!).
>
>> With me so far? Okay, stick with it:
>>
>> 4) On an implementation where char is signed, some char
>> values are negative. (Since the "basic execution set"
>> characters are all non-negative, lackadaisacal testing
>> will not reveal this.)
>
>OK, where does "locale" play into this?
>
>> 5) At least 126 of those negative values are !=EOF.
>
>OK, where does "locale" play into this?
locale doesn't play at all. EOF is a macro. It substitutes exactly
one int value. There are at least 127 negative signed char values. At
least 126 of them cannot equal EOF.
>
>> 6) Therefore, passing one of those char values to a <ctype.h>
>> function invokes undefined behavior.
>
>Which of course, includes exactly the expected behavior as one
>of the possibilities...
And sticking your head in the sand may actually ward off disasters. It
is one of the possible outcomes.
Remove del for email
Undefined behavior always matters if you want you code to work.
Remove del for email
Since you are ignoring advice, and being rude about it, I see no
reason for paying any further attention to you. PLONK.
Yes, it certainly does matter. Passing isspace() an argument that is
not either an int representation of an unsigned char value or the
negative value EOF invokes undefined behavior. If plain char is
signed, then this:
char c = -42;
isspace(c);
invokes undefined behavior. On systems that use an ASCII-based
character set, this can easily happen for, for example, accented
letters.
I regret my earlier attempts to help you.
<snip>
> As far as whether my "system" uses a default signed char,
> in one place in the documentation they state that they USED
> to do it that way, IMPLYING they don't any more, but don't worry
> about it in any event, then in another place they EXPLICITLY
> state that's how they do it currently.
>
> Bastards...
it hardly ever matters...
> > 2) The value of the int argument must be in the range zero
> > through UCHAR_MAX (all positive) or else EOF (negative).
>
> WHO TOL YA DAT!!! WHO TOL YA DAT!!!
"American National Standard
for Information Systems -
Programming Language - C"
known to its friends as ANSI X3.159-1989
also K&R; also Linux man page; also MSDN (though their wording is odd)
google gave 499,000 hits for isspace() I havn't checked them all yet
<snip>
--
Nick Keighley
- Yes it works in practice - but does it work in theory?
> > >>>> An alternative would be to use memmove() here, so you don't
> > >>>> have to do it byte by byte.
> > >>> What would be the actual metrics to make a decision here
> > >>> one way or the other?
I use memcpy() (or memmove()) unless someone proves it's too slow.
Then I try other things.
> > >>> I myself just assume for this kind of stuff
> > >>> that it will only be done on relatively small strings, just a few
> > >>> characters, so it seems that a call to memmove() might be overkill.
it might not. Some compilers insert specially optimised
inline code. Some processors can do memmove() in hardware.
> > >>> Or is it? DOES it depend on the size of the block you'll
> > >>> be moving? How many cycles does it take to assign one
> > >>> character to a previous array position, versus the overhead of
> > >>> a call to memmove(), and the operation of the function itself?
> > >> All this is system-specific.
>
> > > This is not really responsive.
yes it is
> > Valid complaint: Although most of your paragraph was about
> > system-specific nonsense, the actual question "What would be
> > the actual metrics?" can be addressed at least in part without
> > system specificity. Here are some "actual metrics" for your
> > consideration; others may occur to your thought:
>
> > - Brevity
> > - Clarity
> > - Maintainability
> > - Program size
> > - Execution time
> > - Price of tea in China
>
> I am left wondering how you manage to cope with everyday life
> with such poor reading comprehension. I specifically asked about
> "cycles", and you respond with "price of tea"...
I'm left wondering how you manage. Are you usually this rude to
people?
"how many miles to the gallon does it take to travel to London"
"it depends what type of vehicle you use"
"you are unresponsive!"
> > > But I'm "listening"; tell me how many cycles different "systems"
what systems a Cray? a Z80? PC? Marconi Myriad? IBM 360? DeathStation?
(the DS-POT did actually depend on the price of tea in china).
If you really care then measure it. And be aware the answer will
change when you change processor, OS, compiler, compiler version
or compiler switches or omega (that is something I hadn't thought of).
> > > would consume to perform each method...or is memmove() kind
> > > of like fread() and fwrite(), you THINK that they're doing something
> > > "special", but really they're using fgetc() and fputc() which in most
> > > cases you probably could have just used yourself and eliminated
> > > the "middleman"...
the rule is write clearly and use appropriate library functions.
If its too slow or fails to meet some other quality attribute *then*
mofify the code. Always rememebering to measure both before and
afterwards.
> > All this is system-specific.
>
> I was looking for the "Answers Department", but accidentally walked
> into the "Department of Begging the Question" (part of the "Clueless
> Pedants Department", sharing a common desk with the "Pointless
> Arguments Department")...
idiot
<snip>
--
Nick Keighley
"Some people you don't have to parody -- you just quote 'em." -- Tom
Lehrer
Well then the standard is flawed (either chars should always be
unsigned or isdigit should take into account that they may not be).
Instead of forcing people to use defensive programming techniques, why
not just remap the ctype functions for signed char systems, eg:
#if CHAR_MIN < 0
int
isdigit_char_is_signed( int ch )
{
return isdigit( ch ) && ch >= 0;
}
#undef isdigit
#define isdigit isdigit_char_is_signed
// continue with the rest of the ctype functions...
#endif
#if CHAR_MIN < 0
int
isdigit_char_is_signed( int ch )
{
return isdigit( ( unsigned )ch );
IMHO the Standard *is* flawed. And with good reason: the
Committee was chartered to codify existing practice, and the
existing practice for the <ctype.h> functions was flawed. The
moving finger writes, ...
> Instead of forcing people to use defensive programming techniques, why
> not just remap the ctype functions for signed char systems, eg:
>
> #if CHAR_MIN < 0
> int
> isdigit_char_is_signed( int ch )
> {
> return isdigit( ch ) && ch >= 0;
You should make these tests in the opposite order.
Also, this rules out locales that have negative "digits"
in addition to the (positive by definition) '0' through '9':
is it obligatory to call '²' a non-digit? Note that this
technique does not extend to isalpha('ç') or tolower('Ö')
and so on.
This one botches the EOF argument. (IMHO life would be a lot
simpler if the <ctype.h> functions didn't need to handle EOF --
but as I said elsethread, the moving finger writes ...)
what happen on this " "
in the last memmove?
this is my code:
$ cat a.c
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <ctype.h>
char *str_reverse(char *p)
{
char *p1, *p2, ch;
for (p1 = p; *(p1 + 1); p1++) ;
for (p2 = p; p2 < p1; p2++, p1--)
ch = *p2, *p2 = *p1, *p1 = ch;
return p;
}
char *str_trim(char *s)
{
char *p;
p = s + strlen(s) - 1;
while (isspace(*p)) *p-- = '\0';
return s;
}
int main (int argc, char *argv[])
{
printf("\"%s\"\n", argv[1]);
printf("\"%s\"\n", str_trim(argv[1]));
printf("\"%s\"\n",
str_reverse(str_trim(str_reverse(argv[1]))));
return EXIT_SUCCESS;
}
$ make
gcc -ansi -pedantic -Wall -W -g -c -o a.o a.c
a.c:25: warning: unused parameter ‘argc’
gcc a.o -o a.out
$ ./a.out " hello world "
" hello world "
" hello world"
"hello world"
$
<snip>
> this is my code:
>
> $ cat a.c
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <ctype.h>
>
> char *str_reverse(char *p)
> {
> char *p1, *p2, ch;
>
> for (p1 = p; *(p1 + 1); p1++) ;
> for (p2 = p; p2 < p1; p2++, p1--)
> ch = *p2, *p2 = *p1, *p1 = ch;
> return p;
> }
>
> char *str_trim(char *s)
> {
> char *p;
>
> p = s + strlen(s) - 1;
> while (isspace(*p)) *p-- = '\0';
> return s;
> }
>
> int main (int argc, char *argv[])
> {
> printf("\"%s\"\n", argv[1]);
> printf("\"%s\"\n", str_trim(argv[1]));
> printf("\"%s\"\n",
> str_reverse(str_trim(str_reverse(argv[1]))));
> return EXIT_SUCCESS;
> }
[ ... ]
Your program misbehaves when invoked with no arguments or with an empty
string argument.
The REALLY important thing, is what are the defineable cases
for the "hundreds of times" for this "issue"...is it characters that
may appear on a "system", or the "systems" themselves?
> > [...]
> > I WANT MY COOKIE!!!!!!
> >
> You haven't yet shown that char is in fact signed on your
> system.
Anything to weasel out of a bet...
Although you fail to answer the crucial question, by careful
reading between the lines I am now assuming that this "major
bug" is something that only applies to a small, perhaps imaginary,
subset of computer systems...
---
William Ernest Reid
I try to keep an open mind, but not so open my brains fall out...remember,
we actually have a claim on this thread that moving a single character one
space forward can take "a billion" cycles (perhaps Carl Sagan's ghost is
posting here contributing to the pointless obfuscatory pedantry)...
> On some systems, a call to memmove() will generate an actual call to a
> library function that moves one byte at a time just like your loop, and
> thus will always be slower.
OK, we're STILL not up to a "billion" cycles, and I specifically covered
this possibility later in this post. All you're really doing here is
agreeing
with my post (remember, I'm the guy who doesn't want to call memmove()
at all except for large amounts of text). Unfortunately, what you write
is just technical "common sense", and thus is completely inappropriate
for THIS forum, in which there are imaginary "systems" that require a
"billion" cycles to move a single character one space forward...try to
conform to the "rules of the newsgroup", or face the most dire of
consequences...
> On other systems, a call to memmove() will generate an actual call to a
> library function that uses a few (maybe even just one) machine
> instructions to move the bytes much faster than your byte at a time
> loop. Which is faster depends on the number of bytes moved, how much
> overhead there is in a function call, and how much faster the machine
> instructions are than your loop. It is likely that your loop will be
> faster for "short" strings and slower for "long" strings, but exactly
> where the break-even point is will vary from system to system. It might
> be just a few bytes, it might be tens of bytes, it might even be
> hundreds of bytes (although probably not).
OK, I warned you about "common sense", and then you post a
whole bunch more of it...
> On still other systems, a call to memmove() will generate those few (or
> maybe even just one) machine instructions inline, which will almost
> certainly be faster than your byte at a time loop all the time.
Does my turning off ALL optimizations factor into this? I think it
does, since the documentation specifically mentions inlining "intrinsic"
functions as one of the configurable optimization, and I'm pretty
sure memmove() is one of them...
> > or is memmove() kind
> > of like fread() and fwrite(), you THINK that they're doing something
> > "special", but really they're using fgetc() and fputc() which in most
> > cases you probably could have just used yourself and eliminated
> > the "middleman"...
>
> Although the C standard describes fread()/fwrite() as working as if they
> called fgetc()/fputc() for each byte, I've never seen an implementation
> where they actually do that; real implementations nearly always *do* do
> something special instead. On the other hand, I've seen actual
> implementations use all three of the above methods of implementing
> memmove().
OK, I actually use fread() and fwrite() anyway, so I guess I have
wasted any precious cycles THAT way...
---
William Ernest Reid
> >(I wrote)
> >> So to get a good answer, you need to ask in a system-specific news
group.
> >
> > OK, I'll take that as a "I have no idea"...
>
> Take it how you like. It means what it says.
> For the record, I have several answers, depending on which system you're
> interested in. However you have zero chance of getting any of them from
> me now.
I'll take that as "I made a silly statement about it taking 'a billion'
cycles to move a single character one space forward on some 'systems',
and couldn't possibly provide any evidence to support that buffoonery,
therefore I'm now playing the 'temper tantrum card'", one of the more
popular behavior patterns here, where the newsgroup regular takes
faux offense and "stomps off mad" to avoid having to deal with
simple technical issues...
> >all you had
> > to do was pick ANY system (or two or three) that you were familiar with
> > to answer the question...
>
> And had you asked the question over in a system-specific group, I'd have
> answered with a relevant answer.
>
> However its not topical here, so given your abusive attitude I suggest
> you swivel on it.
Brilliant. The best thing about this childish behavior is, of course,
their purported belief that they are somehow withdrawing their valuable
"help" from me and the newsgroup...they're taking their ball and running
home crying, but the ball was deflated in the first place...
---
William Ernest Reid
I'm back with the most-efficient code to do this, except of course
it depends on the "system", with all recommended fixes added in:
char *remove_beg_end_non_text(char *text) {
char *beg;
size_t length;
for(beg=text;isspace(*beg);beg++);
if(*beg=='\0') {
*text='\0';
return text;
}
length=strlen(beg);
while(length>0)
if(!isspace(*(beg+(--length)))) break;
*(beg+(++length))='\0';
return memmove(text,beg,length+1);
}
---
William Ernest Reid
Can't you work it out?
--
Ben.
It was. I've revised the main() function.
$ cat a.c
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <ctype.h>
char *reverse(char *p)
{
char *p1, *p2, ch;
for (p1 = p; *(p1 + 1); p1++) ;
for (p2 = p; p2 < p1; p2++, p1--)
ch = *p2, *p2 = *p1, *p1 = ch;
return p;
}
char *trim(char *s)
{
char *p;
p = s + strlen(s) - 1;
while (isspace(*p)) *p-- = '\0';
return s;
}
int main (int argc, char *argv[])
{
char *s;
if (argc != 2){
printf("Usage: %s <string>\n", argv[0]);
return EXIT_FAILURE;
}
s = malloc(strlen(argv[1]) + 1);
if (!s){
printf("Failed to allocate memory!\n");
return EXIT_FAILURE;
}
strcpy(s, argv[1]);
printf("\"%s\"\n", s);
printf("\"%s\"\n", trim(s));
printf("\"%s\"\n", reverse(trim(reverse(s))));
free(s);
return EXIT_SUCCESS;
}
$ make
gcc -ansi -pedantic -Wall -W -c -o a.o a.c
gcc a.o -o a.out
$ ./a.out
Usage: ./a.out <string>
$ ./a.out ""
""
""
""
$ ./a.out " hello world ! "
" hello world ! "
" hello world !"
"hello world !"
$
I am curious about why you don't put in the cast? Do you not accept
that it should be there?
> if(*beg=='\0') {
>
> *text='\0';
> return text;
> }
>
> length=strlen(beg);
>
> while(length>0)
This test is now redundant
> if(!isspace(*(beg+(--length)))) break;
and this is overly complex. At this point you know for certain that
length > 0 and that there is a non-space character "in front" of
beg[length]. There is no need to test while (length > 0) or to break
when you find it:
while (isspace((unsigned char)beg[--length]));
(I prefer [] to * and + and I want to put the cast in).
> *(beg+(++length))='\0';
>
> return memmove(text,beg,length+1);
> }
--
Ben.
> On Jul 18, 6:17 pm, santosh <santosh....@gmail.com> wrote:
>> lovecreatesbea...@gmail.com wrote:
>> > this is my code:
<snip>
>> > char *str_trim(char *s)
>> > {
>> > char *p;
>>
>> > p = s + strlen(s) - 1;
>> > while (isspace(*p)) *p-- = '\0';
>> > return s;
>> > }
<snip>
>> Your program misbehaves when invoked with no arguments or with an empty
>> string argument.
>
> It was. I've revised the main() function.
But the problem is in the str_trim function, not main:
<snip>
> char *trim(char *s)
> {
> char *p;
>
> p = s + strlen(s) - 1;
You can't do this is strlen(s) is 0.
> while (isspace(*p)) *p-- = '\0';
and this makes matter even worse.
> return s;
> }
--
Ben.
> <snip>
> > I'm back with the most-efficient code to do this, except of course
> > it depends on the "system", with all recommended fixes added in:
> >
> > char *remove_beg_end_non_text(char *text) {
> > char *beg;
> > size_t length;
> >
> > for(beg=text;isspace(*beg);beg++);
>
> I am curious about why you don't put in the cast? Do you not accept
> that it should be there?
Yeah, I haven't received a RATIONAL explanation yet as to why
it MUST be there...it doesn't seem to be needed on MY "system"...
> > if(*beg=='\0') {
> >
> > *text='\0';
> > return text;
> > }
> >
> > length=strlen(beg);
> >
> > while(length>0)
>
> This test is now redundant
I realized that exactly one picosecond after I pressed "Send"...
> > if(!isspace(*(beg+(--length)))) break;
>
> and this is overly complex. At this point you know for certain that
> length > 0 and that there is a non-space character "in front" of
> beg[length]. There is no need to test while (length > 0) or to break
> when you find it:
>
> while (isspace((unsigned char)beg[--length]));
>
> (I prefer [] to * and + and I want to put the cast in).
"Preferences" and "wants" aside, you are correct, sir...
> > *(beg+(++length))='\0';
> >
> > return memmove(text,beg,length+1);
> > }
So that now leaves us with:
char *remove_beg_end_non_text(char *text) {
char *beg;
size_t length;
for(beg=text;isspace(*beg);beg++);
if(*beg=='\0') { *text='\0'; return text; }
length=strlen(beg);
while(!isspace(*(beg+(--length))));
*(beg+(++length))='\0';
return memmove(text,beg,length+1);
}
...with some preferential whitespace adjustments to reduce it
to what appears to be six lines of code...
---
William Ernest Reid
> Ben Bacarisse <ben.u...@bsb.me.uk> wrote in message
> news:87sku78...@bsb.me.uk...
>> "Bill Reid" <horme...@happyhealthy.net> writes:
>
>> <snip>
>
>> > I'm back with the most-efficient code to do this, except of course
>> > it depends on the "system", with all recommended fixes added in:
>> >
>> > char *remove_beg_end_non_text(char *text) {
>> > char *beg;
>> > size_t length;
>> >
>> > for(beg=text;isspace(*beg);beg++);
>>
>> I am curious about why you don't put in the cast? Do you not accept
>> that it should be there?
>
> Yeah, I haven't received a RATIONAL explanation yet as to why
> it MUST be there...it doesn't seem to be needed on MY "system"...
What I am missing is a rational explanation of why you would leave it
out. What do you gain in exchange for this loss of portability?
--
Ben.
I can't imagine a useful program whose overall performance can be measurably
affected by the performance of a string trimming function.
> char *remove_beg_end_non_text(char *text) {
> char *beg;
> size_t length;
>
> for(beg=text;isspace(*beg);beg++);
>
> if(*beg=='\0') {
>
> *text='\0';
> return text;
> }
>
> length=strlen(beg);
>
> while(length>0)
> if(!isspace(*(beg+(--length)))) break;
>
> *(beg+(++length))='\0';
>
> return memmove(text,beg,length+1);
> }
Programer efficiency first. Know thy standard library.
#include <string.h>
char *trim_destructively(char *str)
{
const char *space_chars = " \t\n\v\f\b";
char *skip_space = str + strspn(str, space_chars);
char *skip_nonspace = skip_space + strcspn(skip_space, space_chars);
*skip_nonspace = 0;
return skip_space;
}
memmoving alternative:
char *trim_destructively(char *str)
{
const char *space_chars = " \t\n\v\f\r";
char *skip_space = str + strspn(str, space_chars);
int nonspace_len = strcspn(skip_space, space_chars);
skip_space[nonspace_len] = 0;
return memmove(str, skip_space, nonspace_len + 1);
}
Doh!
> On 2008-07-18, Bill Reid <horme...@happyhealthy.net> wrote:
>> I'm back with the most-efficient code to do this, except of course
>> it depends on the "system", with all recommended fixes added in:
>
> I can't imagine a useful program whose overall performance can be
> measurably affected by the performance of a string trimming function.
>
> [...]
>
> Programer efficiency first. Know thy standard library.
>
> #include <string.h>
>
> [...]
>
> memmoving alternative:
>
> char *trim_destructively(char *str)
> {
> const char *space_chars = " \t\n\v\f\r";
> char *skip_space = str + strspn(str, space_chars);
> int nonspace_len = strcspn(skip_space, space_chars);
> skip_space[nonspace_len] = 0;
> return memmove(str, skip_space, nonspace_len + 1);
> }
>
> Doh!
What is the reason for ``int'' as the return value of ``strcspn()'' since
the standard specifies a return type of ``size_t'' for that function.
Both functions had problems. Revised:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <ctype.h>
char *reverse(char *p)
{
char *p1, *p2, ch;
if (!*p || !p) return p;
for (p1 = p; *(p1 + 1); p1++) ;
for (p2 = p; p2 < p1; p2++, p1--)
ch = *p2, *p2 = *p1, *p1 = ch;
return p;
}
char *trim(char *s)
{
char *p;
if (!*s || !s) return s;
p = s + strlen(s) - 1;
while (isspace(*p)) *p-- = '\0';
return s;
}
int main (int argc, char *argv[])
That would transform " foo bar " to "foo".
Ike
Now turn it around. Make the trim function remove the leading
blanks, rather than trailing blanks (much simpler), and adjust the
rest accordingly. The basic action will be:
if (s)
while (' ' == *p) p++;
return p;
This is an exercise, since in practice the added efficiency won't
matter.
--
[mail]: Chuck F (cbfalconer at maineline dot net)
[page]: <http://cbfalconer.home.att.net>
Try the download section.
They both still do.
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <ctype.h>
>
> char *reverse(char *p)
> {
> char *p1, *p2, ch;
>
> if (!*p || !p) return p;
There is no point in testing p after *p. You want:
if (!p || !*p) return p;
or NULL will still not be handled correctly. (You are not obliged to
handle NULL -- I often leave it as part of the contract between caller
and callee that NULL will not be passed -- but you seem to want to
handle a NULL correctly or you would not have tested for it at all.)
> for (p1 = p; *(p1 + 1); p1++) ;
> for (p2 = p; p2 < p1; p2++, p1--)
> ch = *p2, *p2 = *p1, *p1 = ch;
> return p;
> }
>
> char *trim(char *s)
> {
> char *p;
>
> if (!*s || !s) return s;
See above.
> p = s + strlen(s) - 1;
> while (isspace(*p)) *p-- = '\0';
Here you can still generate in invalid pointer. All pointer/array
loops that run backwards need extra care in C. Think of --
(particularly postfix --) as a red flag the signals danger.
> return s;
> }
--
Ben.
More responses.
--
Eric Sosman
eso...@ieee-dot-org.invalid
On Jul 19, 7:45 pm, Ben Bacarisse <ben.use...@bsb.me.uk> wrote:
> "lovecreatesbea...@gmail.com" <lovecreatesbea...@gmail.com> writes:
> > char *reverse(char *p)
> > {
> > char *p1, *p2, ch;
>
> > if (!*p || !p) return p;
>
> There is no point in testing p after *p. You want:
>
> if (!p || !*p) return p;
The order doesn't matter if the test against p is trivial:) I'd like
to keep it and take you suggestion. My code then becomes the same as
the one in Jens' post at the beginning of this thread.
> or NULL will still not be handled correctly. (You are not obliged to
> handle NULL -- I often leave it as part of the contract between caller
> and callee that NULL will not be passed -- but you seem to want to
> handle a NULL correctly or you would not have tested for it at all.)
Yes, even p1 doesn't equal to NULL, it's not a valid pointer
immediately.
char *p1, *p2 = NULL;
> > char *trim(char *s)
> > {
> > char *p;
>
> > if (!*s || !s) return s;
>
> See above.
>
> > p = s + strlen(s) - 1;
> > while (isspace(*p)) *p-- = '\0';
>
> Here you can still generate in invalid pointer. All pointer/array
> loops that run backwards need extra care in C. Think of --
> (particularly postfix --) as a red flag the signals danger.
Do you mean pointing a pointer at an address doesn't belong to one's
own code isn't guaranteed even if no dereference occured. Code
Revised below,
#include <stdlib.h>
#include <string.h>
#include <ctype.h>
/* reverse a string */
char *reverse(char *s)
{
char *p1, *p2, c;
if (!s || !*s) return s;
for (p1 = s; *(p1 + 1); p1++) ;
for (p2 = s; p2 < p1; p2++, p1--)
c = *p2, *p2 = *p1, *p1 = c;
return s;
}
/* trim the trailing spaces of a string */
char *trim(char *s)
{
char *p;
if (!s || !*s) return s;
for (p = s + strlen(s) - 1; p != s; p--)
if (isspace(*p)) *p = '\0';
if (isspace(*p)) *p = '\0';
return s;
}
#define trim rtrim /* trim() is synonomous with rtrim() */
/* Remove trailing whitespace */
char * rtrim(char *str) {
char *s, *p; int c;
s = p = str;
while ((c = *s++)) if (c > ' ') p = s;
*p = '\0';
return str;
}
/* Remove leading whitespace */
char * ltrim(char *str) {
char *s, *p;
for (s = p = str; *s && *s <= ' '; s++) ;
while ((*p++ = *s++)) ;
return str;
}
/* Combination of the two */
char * alltrim(char *str) {
return rtrim(ltrim(str));
}
--
Joe Wright
"Everything should be made as simple as possible, but not simpler."
--- Albert Einstein ---
I also think it is. It doesn't write such a test on an extra line ``if
(!s || !*s) return s;'', and doesn't have multiple exit points.
will these be better?
> while ((c = *s++)) if (c > ' ') p = s;
c != ' '
> for (s = p = str; *s && *s <= ' '; s++) ;
*s == ' ';
>Thank santosh, Ben and the Group
>
>On Jul 19, 7:45 pm, Ben Bacarisse <ben.use...@bsb.me.uk> wrote:
>> "lovecreatesbea...@gmail.com" <lovecreatesbea...@gmail.com> writes:
>> > char *reverse(char *p)
>> > {
>> > char *p1, *p2, ch;
>>
>> > if (!*p || !p) return p;
>>
>> There is no point in testing p after *p. You want:
>>
>> if (!p || !*p) return p;
>
>The order doesn't matter if the test against p is trivial:) I'd like
>to keep it and take you suggestion. My code then becomes the same as
>the one in Jens' post at the beginning of this thread.
The order certainly does matter. If p is NULL, which is one of the
things you are checking, then *p invokes undefined behavior. Ben's
code takes advantage of the short circuit nature of || and if p is
NULL then *p is never evaluated.
Remove del for email
The N1124 May 2005 draft shows that isspace() accepts an int. How is
an negative value an out-of-range argument?
> Use `while (isspace( (unsigned char)str[i] ))'. (This
> is one of those occasions where a cast is almost always required
> and almost always omitted, as opposed to the more usual situation
> where a cast is almost always inserted and almost always wrong.)
The C-FAQ 13.6 <http://c-faq.com/lib/strtok.html> shows two example
calls on isspace() with argument type of char without any cast.
I still don't understand why a cast is needed! stupid me. Can someone
please inspire me?