find character within received string

C

cerr

Hi,

I'm writing an FTP xclient and in order to receive data, I need to figure out contents of the "PASV" reply, so I came up with following:

....
....
printf ("MESSAGE FROM SERVER (PASV):\n%s\n", replyBuf);
// calculate port number
if(pch = strchr(replyBuf,')') != NULL) { //find end of response string
strend = pch - replyBuf;
printf("end of resp str: %d\n",strend);
....
....
and for some reason I get this result:

MESSAGE FROM SERVER (PASV):
227 Entering Passive Mode (3,94,213,53,84,75).
!ü·P%í¤%í@%îðà!?"iÔÌ"ià
%í£À
end of resp str: -636416131

Why would that be? strchr() "returns a pointer to the first occurrence of character in the C string str." - so where am I going wrong here?

Thank you!
Ron
 
B

Ben Bacarisse

cerr said:
I'm writing an FTP xclient and in order to receive data, I need to
figure out contents of the "PASV" reply, so I came up with following:
...
...
printf ("MESSAGE FROM SERVER (PASV):\n%s\n", replyBuf);
// calculate port number
if(pch = strchr(replyBuf,')') != NULL) { //find end of response string

Missing parentheses -- an easy mistake because the literal ')' makes it
a bit harder to read. You are assigning 1 to pch (the result of the !=
test).
strend = pch - replyBuf;
printf("end of resp str: %d\n",strend);

<snip>
 
E

Eric Sosman

Hi,

I'm writing an FTP xclient and in order to receive data, I need to figure out contents of the "PASV" reply, so I came up with following:

...
...
printf ("MESSAGE FROM SERVER (PASV):\n%s\n", replyBuf);
// calculate port number
if(pch = strchr(replyBuf,')') != NULL) { //find end of response string

This doesn't do what you think it does. Simplifying the
pieces a bit to clarify, you've got

if ( X = Y != NULL )

and because the != inequality operator "binds more tightly than"
the = assignment operator, this is equivalent to

if ( X = ( Y != NULL ) )

whereas what you really wanted was

if ( ( X = Y ) != NULL )

Insert those parentheses in the original, and you've got

if ( (pch = strchr(replyBuf, ')')) != NULL) { //find...

which you'll probably find much more satisfactory.

Many compilers will issue warnings about this construct,
because although the original is "legal" it is almost never
what was meant. For gcc you can use the -Wparentheses command-
line flag to enable the warning; better yet, use -Wall (which
enables -Wparentheses and several other helpful warnings), or
better still use -Wall -Wextra (because the "all" in -Wall is a
misnomer).
 
K

Keith Thompson

Ben Bacarisse said:
Missing parentheses -- an easy mistake because the literal ')' makes it
a bit harder to read. You are assigning 1 to pch (the result of the !=
test).


<snip>

cerr said in a followup that pch is of type char*. Attempting to assign
an int value to a char* *should* produce a diagnostic. If it didn't,
crank up your compiler's warning level and pay attention to any warnings
it produces. (IMHO that particular error should be treated as fatal,
not as a warning, but gcc in particular likes to warn about some
constraint violations.)
 
C

cerr

This doesn't do what you think it does. Simplifying the

pieces a bit to clarify, you've got



if ( X = Y != NULL )



and because the != inequality operator "binds more tightly than"

the = assignment operator, this is equivalent to



if ( X = ( Y != NULL ) )



whereas what you really wanted was



if ( ( X = Y ) != NULL )



Insert those parentheses in the original, and you've got



if ( (pch = strchr(replyBuf, ')')) != NULL) { //find...



which you'll probably find much more satisfactory.



Many compilers will issue warnings about this construct,

because although the original is "legal" it is almost never

what was meant. For gcc you can use the -Wparentheses command-

line flag to enable the warning; better yet, use -Wall (which

enables -Wparentheses and several other helpful warnings), or

better still use -Wall -Wextra (because the "all" in -Wall is a

misnomer).



--

Eric Sosman

(e-mail address removed)

Yep, missing Paranthesis was it! Added those fixed the issue!
Thank everyone who chimed in!

Ron
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,744
Messages
2,569,479
Members
44,899
Latest member
RodneyMcAu

Latest Threads

Top