strlen(), signed vs unsigned & some ptr questions

[Zach]

I am getting 3 warnings I don't fully understand. My program works as
expected however I'd like to clear up these warnings and fix my code.

Here are the 3 warnings:
(I am compiling under gcc with "-W -Wall -std=c99 -pedantic")

count-tokens.c: In function ‘return_tokens’:
count-tokens.c:25: warning: comparison between signed and unsigned
count-tokens.c:27: warning: assignment makes pointer from integer
without a cast
count-tokens.c:29: warning: comparison between pointer and integer

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

int numtokens = 0;
char *p;
char string[10] = "1 2 3 4 5";
int i;

int return_tokens(void);

int main(void)
{
printf("string length: %d\n",strlen(string));

return_tokens();
printf("Number of tokens: %d\n",numtokens+1);


return EXIT_SUCCESS;
}

int return_tokens(void){

for (i=0; i < (strlen(string) + 1); i++)
{
p = string;

if (p == ' ')
{
numtokens++;
}
else
{
p++;
}
}

return numtokens;

}

Regards,
Zach

 

[BartC]

I am getting 3 warnings I don't fully understand. My program works as
expected however I'd like to clear up these warnings and fix my code.

Here are the 3 warnings:
(I am compiling under gcc with "-W -Wall -std=c99 -pedantic")

char *p;

if (p == ' ')

You probably meant *p==' ' here.

 

[david]

I am getting 3 warnings I don't fully understand. My program works as
expected however I'd like to clear up these warnings and fix my code.

Here are the 3 warnings:
(I am compiling under gcc with "-W -Wall -std=c99 -pedantic")

count-tokens.c: In function ‘return_tokens’:
count-tokens.c:25: warning: comparison between signed and unsigned
count-tokens.c:27: warning: assignment makes pointer from integer
without a cast
count-tokens.c:29: warning: comparison between pointer and integer

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

int numtokens = 0;
char *p;
char string[10] = "1 2 3 4 5";
int i;

int return_tokens(void);

int main(void)
{
        printf("string length: %d\n",strlen(string));

        return_tokens();
        printf("Number of tokens: %d\n",numtokens+1);

        return EXIT_SUCCESS;

}

int return_tokens(void){

        for (i=0; i < (strlen(string) + 1); i++)
                {
                        p = string;

                        if (p == ' ')
                                {
                                        numtokens++;
                                }
                        else
                                {
                                        p++;
                                }
                }

        return numtokens;

}

ok, I try to answer with my poor english

Line 25:
strlen returns unsigned int, i is declared as int, cast the return
value or don't use -W

Line 27:
a pointer must point to an address, not to a char

Line 29
here you compare a pointer to char with a char

why are you increasing numtokens?
printf("Number of tokens: %d\n",numtokens+1);
this gives you one more than real tokens
and ... you are using global variables unnecesarly, here is your code
corrected:

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

int numtokens = 0;
char *p;
char string[10] = "1 2 3 4 5";
int i;

int return_tokens(void);

int main(void)
{
printf("string length: %d\n",strlen(string));

return_tokens();
printf("Number of tokens: %d\n",numtokens+1); /* Perhaps wrong
increasing numtokens? */

return EXIT_SUCCESS;

}

int return_tokens(void){

for (i=0; i < ((int)strlen(string) + 1); i++)
{
p = &string;

if (*p == ' ')
{
numtokens++;
}
else
{
p++;
}
}

return numtokens;

}

A better way to do the same:

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

char string[10] = "1 2 3 4 5";

int return_tokens(char *p);

int main(void)
{
int i;

printf("string length: %d\n", strlen(string));
i = return_tokens(string);
printf("Number of tokens: %d\n", i);

return EXIT_SUCCESS;
}

int return_tokens(char *p)
{
int numtokens = 0;

while (*p) {
if (*p == ' ') numtokens++;
p++;
}
return numtokens;
}

 

[James Kuyper]

I am getting 3 warnings I don't fully understand. My program works as
expected however I'd like to clear up these warnings and fix my code.

Here are the 3 warnings:
(I am compiling under gcc with "-W -Wall -std=c99 -pedantic")

count-tokens.c: In function ‘return_tokens’:
count-tokens.c:25: warning: comparison between signed and unsigned

strlen() returns size_t. If the conversion rank of size_t is greater
than that of int, or if INT_MAX < SIZE_MAX, such comparisons are carried
by first converting the int value to size_t, a conversion that can
convert negative values into large positive values, such that -1 >
(size_t)5. Since negative values will not be involved in this particular
case, you shouldn't run into problems. However, you can turn off this
warning by declaring i unsigned.

count-tokens.c:27: warning: assignment makes pointer from integer
without a cast

p is a pointer to a char, it is not a char, which makes it pretty much
pointless to store an integer value like string in that pointer. What
you meant was

*p = string;

count-tokens.c:29: warning: comparison between pointer and integer

Here, again, you're comparing a pointer with an integer, something that
is virtually pointless. You should have written:

if(*p == ' ')

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

int numtokens = 0;
char *p;
char string[10] = "1 2 3 4 5";
int i;

int return_tokens(void);

int main(void)
{
printf("string length: %d\n",strlen(string));

return_tokens();
printf("Number of tokens: %d\n",numtokens+1);


return EXIT_SUCCESS;
}

int return_tokens(void){

for (i=0; i < (strlen(string) + 1); i++)
{
p = string;

if (p == ' ')
{
numtokens++;
}
else
{
p++;
}
}

return numtokens;

}

Regards,
Zach

 

[Flash Gordon]

I am getting 3 warnings I don't fully understand. My program works as
expected however I'd like to clear up these warnings and fix my code.

Here are the 3 warnings:
(I am compiling under gcc with "-W -Wall -std=c99 -pedantic")

count-tokens.c: In function ‘return_tokens’:
count-tokens.c:25: warning: comparison between signed and unsigned
count-tokens.c:27: warning: assignment makes pointer from integer
without a cast
count-tokens.c:29: warning: comparison between pointer and integer

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

int numtokens = 0;
char *p;
char string[10] = "1 2 3 4 5";
int i;

Why are the above not declared locally in appropriate places? There are
sometimes good reasons to use "global" variables, but not in this case.

int return_tokens(void);

int main(void)
{
printf("string length: %d\n",strlen(string));

return_tokens();
printf("Number of tokens: %d\n",numtokens+1);

return EXIT_SUCCESS;

}

int return_tokens(void){

for (i=0; i < (strlen(string) + 1); i++)
{
p = string;

if (p == ' ')
{
numtokens++;
}
else
{
p++;
}
}

return numtokens;

}

ok, I try to answer with my poor english

Line 25:
strlen returns unsigned int

strlen returns a size_t which an unsigned type, and on your system
happens to be unsigned int, it could be unsigned long (for example) on
another system.

, i is declared as int, cast the return
value or don't use -W

Declaring i as a local variable of type size_t would be better.

Line 27:
a pointer must point to an address, not to a char

This is probably poor English, rather than miss-understanding. However...

A pointer contains an address, in general it does not point to one
(pointers to pointers excepted). In this case the OP probably meant to
get the address of string (i.e. &string or string+i) rather than
the character.

Line 29
here you compare a pointer to char with a char

why are you increasing numtokens?
printf("Number of tokens: %d\n",numtokens+1);
this gives you one more than real tokens
and ... you are using global variables unnecesarly, here is your code
corrected:

You are still using globals variables unnecessarily...

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

int numtokens = 0;
char *p;
char string[10] = "1 2 3 4 5";
int i;

So delete all four of the lines above...

int return_tokens(void);

int return_tokens(char *str);

int main(void)
{

char string[] = "1 2 3 4 5";
int numtokens;

Note I don't bother to specify the size of the array, I let the compiler
work it out.

printf("string length: %d\n",strlen(string));

%d expects an int, not some unsigned type.
printf("string length: %lu\n",(unsigned long)strlen(string));
The cast is because you still don't know which particular unsigned type
it is. In C99 you could use %zu and drop the cast.

return_tokens();

numtokens = return_tokens(string);
Using the parameter passing and return value mechanisms provided by C
rather than global variables.

printf("Number of tokens: %d\n",numtokens+1); /* Perhaps wrong
increasing numtokens? */

return EXIT_SUCCESS;

Whilst the above is correct, I would use 0 instead since it also is
defined as meaning success and is considerably less to type.

}

int return_tokens(void){

int return_tokens(char *str)
{
size_t i;
char *p;
int numtokens;

for (i=0; i < ((int)strlen(string) + 1); i++)

Now we can drop the cast without introducing a warning
for (i=0, numtokens=0; i < strlen(str) + 1; i++)

{
p = &string;

if (*p == ' ')
{
numtokens++;
}
else
{
p++;

Why bother incrementing p? It gets assigned a new value anyway on the
next time round the loop. In fact, we could get rid of p all together
and use
if (str == ' ')
numtokens++;

}
}

return numtokens;

}

A better way to do the same:

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

char string[10] = "1 2 3 4 5";

int return_tokens(char *p);

int main(void)
{
int i;

printf("string length: %d\n", strlen(string));
i = return_tokens(string);
printf("Number of tokens: %d\n", i);

return EXIT_SUCCESS;
}

int return_tokens(char *p)
{
int numtokens = 0;

while (*p) {
if (*p == ' ') numtokens++;
p++;
}
return numtokens;
}

OK, you've corrected most of the things I address above, so treat my
comments as comments to the OP as to why things needed changing. There
is still the problem of printing a size_t with %d, and I still don't
like the style of return_tokens. I would prefer soething like

int return_tokens(char *str)
{
int numtokens;
for (numtokens = *p?1:0; *p; p++)
if (*p)
numtokens++;
}


Note I treat an empty string as having no tokens, but a non-empty string
has at least one token. Also there are the question of consecutive
spaces or a space at the start of the string which are not addressed.
I think the +1 in the original code was an attempt to deal with the fact
the original counted the number of token separators (i.e. spaces) rather
than the number of tokens so returned one too few (except for an empty
string).

 

[Spiros Bousbouras]

int return_tokens(void){

for (i=0; i < (strlen(string) + 1); i++)

Apart from the comments made by others, what you have here is
inefficient because you call strlen() on every repetition of the
loop although the string doesn't change. It's better to do

size_t string_length = strlen(string) ;
for (i=0; i <= string_length; i++)

< snip more code >

 

[Spiros Bousbouras]

I would prefer soething like

int return_tokens(char *str)
{
int numtokens;
for (numtokens = *p?1:0; *p; p++)
if (*p)
numtokens++;

}

I might prefer it too if p was declared, the function actually
returned a value and the if (*p) part were to change to
if (*p == ' ')

Well actually no , I find the for initialisation cryptic and
error prone. I'd much rather write
if ( *p == 0 ) return 0;
for (numtokens = 1 ; *p ; p++)

 

[Flash Gordon]

I might prefer it too if p was declared, the function actually
returned a value and the if (*p) part were to change to
if (*p == ' ')

OK, I failed to change the name all the way through.

Well actually no , I find the for initialisation cryptic and
error prone. I'd much rather write
if ( *p == 0 ) return 0;
for (numtokens = 1 ; *p ; p++)

<snip>

Yes, that I would also consider.

 

[CBFalconer]

I am getting 3 warnings I don't fully understand. My program works as
expected however I'd like to clear up these warnings and fix my code.

Here are the 3 warnings:
(I am compiling under gcc with "-W -Wall -std=c99 -pedantic")

count-tokens.c: In function ‘return_tokens’:
count-tokens.c:25: warning: comparison between signed and unsigned
count-tokens.c:27: warning: assignment makes pointer from integer
without a cast
count-tokens.c:29: warning: comparison between pointer and integer

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

int numtokens = 0;
char *p;
char string[10] = "1 2 3 4 5";
int i;
.... snip ...

int return_tokens(void){

for (i=0; i < (strlen(string) + 1); i++) {

i is an int. strlen is a size_t, which is unsigned.

p = string;

p is a pointer to char. string is a char, and loaded as an int.

if (p == ' ') {

p is a pointer to char. ' ' is an integer.

numtokens++;
}
else {
p++;
}
}
return numtokens;
}

You should mark the lines that correspond to the warnings/errors.
I had to read the code to find them.

Avoid using globals.

 

[Anand Hariharan]

int return_tokens(void){

        for (i=0; i < (strlen(string) + 1); i++)
                {
                        p = string;

                        if (p == ' ')
                                {
                                        numtokens++;
                                }
                        else
                                {
                                        p++;
                                }
                }

        return numtokens;

}

Suggest you look up strchr, and reconsider implementing return_tokens
based on strchr.

- Anand

 

[Richard Bos]

c3RyaW5nKSArIDEpOyBpKyspCj4goCCgIKAgoCCgIKAgoCCgIHsKPiCgIKAgoCCgIKAgoCCgIKAg
oCCgIKAgoCBwID0gc3RyaW5nW2ldOwo+Cj4goCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgaWYgKHAg
PT0gJyAnKQo+IKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgoCCgIKAgewo+IKAgoCCgIKAgoCCg

Ik koop deze tabakszaak niet - er zit een kras op.

Richard

 

[Anand Hariharan]

Ik koop deze tabakszaak niet - er zit een kras op.

My Pig Latin is rusty, but thank you for pointing out that either
Google
or my browser landed up encoding my post. It shows up fine on Google
though.

Hoping that this post won't get treated the same way as well (did
experiment with a post in alt.test prior to posting here).

- Anand