Problem with application on Windows x64

[P]

Hi all,

I have compiled a C application (japach:
http://www.jikos.cz/jikos/japach/) under cygwin on Windows NT. It
works fine on all versions of win32. When I ran it on Windows x64, it
hangs on the following line:

strncpy(buffer2, client.host_name, MAXLINE - strlen(buffer2));

The problem is client.host_name, which is a string. This line is
reached after a few nested calls to different functions. In main,
where client.host_name is set, I could output it just fine. When it
gets to this function, it just hangs there.

Shouldn't this work on Windows x64? Winx64 has WOW that allows 32-bit
applications to be run. Is there something I'm not aware of?

Thanks.

Patrick

 

[David Resnick]

Hi all,

I have compiled a C application (japach:
http://www.jikos.cz/jikos/japach/) under cygwin on Windows NT. It
works fine on all versions of win32. When I ran it on Windows x64, it
hangs on the following line:

strncpy(buffer2, client.host_name, MAXLINE - strlen(buffer2));

The problem is client.host_name, which is a string. This line is
reached after a few nested calls to different functions. In main,
where client.host_name is set, I could output it just fine. When it
gets to this function, it just hangs there.

Dunno about the 32 vs 64 bit issue.

As "C" that line looks very odd to me. Why does what you are copying
to it depend on the strlen of the current contents?!? Often the target
of a strcpy/strncpy has garbage in it. Consider if that garbage has no
NUL characters for > MAXLINE bytes.... You get a very large number.
And strncpy is the wrong tool for almost any task.

-David

 

[Walter Roberson]

strncpy(buffer2, client.host_name, MAXLINE - strlen(buffer2));

strncpy() would normally be used to write into its first argument,
buffer2 in this case.

Has buffer2 been initialized at that point? If it has not been,
then when you take strlen(buffer2) in the third parameter, you
could get just about anything for the length as strlen() will keep
looking past the end of buffer2 until it happens to find a '\0'.

If buffer2 has indeed been initialized at that point, then
what is the point of making the maximum copying length dependant
on its current contents, when you are overwriting those contents?

 

[P]

strncpy() would normally be used to write into its first argument,
buffer2 in this case.

Has buffer2 been initialized at that point? If it has not been,
then when you take strlen(buffer2) in the third parameter, you
could get just about anything for the length as strlen() will keep
looking past the end of buffer2 until it happens to find a '\0'.

If buffer2 has indeed been initialized at that point, then
what is the point of making the maximum copying length dependant
on its current contents, when you are overwriting those contents?
--
"It is important to remember that when it comes to law, computers
never make copies, only human beings make copies. Computers are given
commands, not permission. Only people can be given permission."
-- Brad Templeton

I'm not sure what the intend of the developer was. A few lines above
that, there's this:

strcpy(buffer2, "");

So strlen(buffer2) is 0 when it gets to that statement where it hangs.
This shouldn't cause the app to hang, should it?

Patrick

 

[Richard Heathfield]

P said:

I'm not sure what the intend of the developer was. A few lines above
that, there's this:

strcpy(buffer2, "");

So strlen(buffer2) is 0 when it gets to that statement where it hangs.
This shouldn't cause the app to hang, should it?

It depends on how large an array buffer2 represents. If buffer2 has fewer
than MAXLINE characters available, the buffer will be overrun, and that
could certainly cause the program to exhibit serious problems, which may
well include "hanging".

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: normal service will be restored as soon as possible. Please do not
adjust your email clients.

 

[David Resnick]

I'm not sure what the intend of the developer was. A few lines above
that, there's this:

strcpy(buffer2, "");

So strlen(buffer2) is 0 when it gets to that statement where it hangs.
This shouldn't cause the app to hang, should it?

No. strncpy is still a dodgy thing here, because if MAXLINE characters
are copied it will result in a non-NUL-terminated string. Having
peeked at the source, MAXLINE is 2048. Even if MAXLINE characters are
not copied, it is wasteful -- it means for a 48 character hostname
there is a side effect of a 2000 byte memset. Which is legal, just
silly.

I suspect the client.host_name contents. The code where I see that
assigned is this:

if (NULL !=
(hostentry =
gethostbyaddr((char *) &cli_addr.sin_addr,
sizeof(cli_addr.sin_addr),
AF_INET)))
client.host_name = hostentry->h_name;
else
client.host_name =
inet_ntoa(cli_addr.sin_addr)

Forgetting the (off topic) platform specific details of what the above
POSIX/BSD functions do, I'd guess that this might be fixed by copying
the values placed in the host_name field (and handling the memory
management, or having it be a character array in the struct). Just a
guess though...

-David

 

[Jordan Abel]

Dunno about the 32 vs 64 bit issue.

As "C" that line looks very odd to me. Why does what you are copying
to it depend on the strlen of the current contents?!? Often the target
of a strcpy/strncpy has garbage in it. Consider if that garbage has no
NUL characters for > MAXLINE bytes.... You get a very large number.
And strncpy is the wrong tool for almost any task.

It looks like he wants strcat.

 

[P]

No. strncpy is still a dodgy thing here, because if MAXLINE characters
are copied it will result in a non-NUL-terminated string. Having
peeked at the source, MAXLINE is 2048. Even if MAXLINE characters are
not copied, it is wasteful -- it means for a 48 character hostname
there is a side effect of a 2000 byte memset. Which is legal, just
silly.

I suspect the client.host_name contents. The code where I see that
assigned is this:

if (NULL !=
(hostentry =
gethostbyaddr((char *) &cli_addr.sin_addr,
sizeof(cli_addr.sin_addr),
AF_INET)))
client.host_name = hostentry->h_name;
else
client.host_name =
inet_ntoa(cli_addr.sin_addr)

Forgetting the (off topic) platform specific details of what the above
POSIX/BSD functions do, I'd guess that this might be fixed by copying
the values placed in the host_name field (and handling the memory
management, or having it be a character array in the struct). Just a
guess though...

-David

David,

Thanks! That fixed it. But I'm not sure why it works. Could you
explain it to me?

This is what I've changed:

- in client struct (japach.h), from char *host_name; to char
host_name[MAXLINE];
- in japach.c:

if (NULL !=
(hostentry =
gethostbyaddr((char *) &cli_addr.sin_addr,
sizeof(cli_addr.sin_addr),
AF_INET)))
//client.host_name = hostentry->h_name;
strcpy(client.host_name,
hostentry->h_name);
else
//client.host_name =
// inet_ntoa(cli_addr.sin_addr)
strcpy(client.host_name,
inet_ntoa(cli_addr.sin_addr) );

Thanks again!

Patrick

 

[David Resnick]

Forgetting the (off topic) platform specific details of what the above
POSIX/BSD functions do, I'd guess that this might be fixed by copying
the values placed in the host_name field (and handling the memory
management, or having it be a character array in the struct). Just a
guess though...

-David

David,

Thanks! That fixed it. But I'm not sure why it works. Could you
explain it to me?

This is what I've changed:

- in client struct (japach.h), from char *host_name; to char
host_name[MAXLINE];
- in japach.c:

if (NULL !=
(hostentry =
gethostbyaddr((char *) &cli_addr.sin_addr,
sizeof(cli_addr.sin_addr),
AF_INET)))
//client.host_name = hostentry->h_name;
strcpy(client.host_name,
hostentry->h_name);
else
//client.host_name =
// inet_ntoa(cli_addr.sin_addr)
strcpy(client.host_name,
inet_ntoa(cli_addr.sin_addr) );

As a general rule, it is a bad idea to keep a pointer to a string which
is not guaranteed to be unchanged if that is what one needs. Rather it
is better to copy the string into something one owns. <OT>Both
gethostbyaddr and inet_ntoa return a pointer to static storage that can
be overwritten. Good idea is to use safer versions of these functions
or to copy the result.</OT>

-David

 

[P]

As a general rule, it is a bad idea to keep a pointer to a string which

is not guaranteed to be unchanged if that is what one needs. Rather it
is better to copy the string into something one owns. <OT>Both
gethostbyaddr and inet_ntoa return a pointer to static storage that can
be overwritten. Good idea is to use safer versions of these functions
or to copy the result.</OT>

-David

David,

What are the safer versions of gethostbyaddr and inet_ntoa?

Patrick

 

[Flash Gordon]

What are the safer versions of gethostbyaddr and inet_ntoa?

The <OT></OT> tags above meant that the advice is off topic and you
should ask for further information else where. If this is the first time
you have come across this you can be forgiven. However, please ask about
those functions and alternative on a group where they are topical,
comp.unix.programmer springs to mind as one possibility, but make sure
you check their archives and FAQ to find out if the question has already
been asked and answered and, indeed, if it is topical there.

 

[Kenny McCormack]

P jokingly wrote: ....

The <OT></OT> tags above meant that the advice is off topic and you
should ask for further information else where. If this is the first time
you have come across this you can be forgiven.

I think you need to have your sarcasm detector recalibrated.