strncpy() fails to copy

[arnuld]

WANTED: To copy some fixed number of characters in one array from another
using strncpy

WHAT I GOT: Its fails to copy

WHAT I WANT TO KNOW: I know where is the problem (SIZE_SERVER_PORT). I
just want to know why changing the size of one array affects the other
array when both arrays are being used independently from each other ?
(Just change SIZE_SERVER_PORT from 5 to 6 and you will see)


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

#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>


enum {SIZE_SERVER_IP = 16, SIZE_SERVER_PORT = 5};


int socket_connect(const char* ip, const char* port );


int main(void)
{
if( socket_connect("192.168.0.228", "65534") < 0 )
{
printf("IN: %s at %d: Socket Error\n", __FILE__, __LINE__);
return -1;
}

return 0;
}


int socket_connect(const char* ip, const char* port )
{
char servIP[SIZE_SERVER_IP] = {0};
char myport[SIZE_SERVER_PORT] = {0};
int servPort = -1;
int sockfd = -1;
int c ;

struct sockaddr_in server,client;

if( NULL == ip || NULL == port )
{
printf("IN: %s, %s | could not connect to server, either IP or PORT
are NULL or invalid, please check this function first\n", __func__,
__FILE__);
return -1;
}
else
{
printf("Server IP = %s, strlen(IP) = %d\n", ip, strlen(ip));
printf("Server PORT = %s\n", port);
}

strncpy(servIP, ip, SIZE_SERVER_IP);

strcpy(myport, port);
servPort=atoi(myport);

printf("Copied Server IP = %s\n", servIP);
printf("Copied Server PORT = %d\n", servPort);

if(servPort < 0 || NULL == servIP)
{
printf("IN: %s, %s | Invalid PORT/IP\n", __func__, __FILE__);
return -1;
}

memset((char*)&client,0,sizeof(struct sockaddr_in));
memset((char*)&server,0,sizeof(struct sockaddr_in));

sockfd=socket(AF_INET,SOCK_STREAM,0);
if(sockfd<0)
{
printf("IN: %s, %s | socket call failed:%s\n", __func__, __FILE__,
strerror(errno));
return -1;
}

server.sin_family=AF_INET;
server.sin_port=htons(servPort);

if(inet_pton(AF_INET, servIP, &server.sin_addr) <= 0)
{
printf("IN: %s - %s: INET_PTON ERROR for string: %s\n", __func__,
__FILE__, servIP);
}

c = connect(sockfd,(struct sockaddr *)&server,sizeof(struct
sockaddr_in));
if(c < 0)
{
printf("IN: %s, %s | connect call failed, connect returned = %d\n",
__func__, __FILE__, c);
printf("STRERROR(errono) = %s\n", strerror(errno));
return -1;
}

return sockfd;
}

==================== OUTPUT =============================
[arnuld@dune programs]$ gcc -ansi -pedantic -Wall -Wextra socket-test.c
[arnuld@dune programs]$ ./a.out
Server IP = 192.168.0.228, strlen(IP) = 13
Server PORT = 65534
Copied Server IP =
Copied Server PORT = 65534
IN: socket_connect - socket-test.c: INET_PTON ERROR for string:
IN: socket_connect, socket-test.c | connect call failed, connect returned
= -1
STRERROR(errono) = Connection refused
IN: socket-test.c at 21: Socket Error
[arnuld@dune programs]$

 

[Mark Bluemel]

arnuld wrote:
....

> enum {SIZE_SERVER_IP = 16, SIZE_SERVER_PORT = 5}; ....
int main(void)
{
if( socket_connect("192.168.0.228", "65534") < 0 )

How many characters in that second argument?
....

int socket_connect(const char* ip, const char* port )
{
char servIP[SIZE_SERVER_IP] = {0};

How many characters here?

char myport[SIZE_SERVER_PORT] = {0}; ....

strncpy(servIP, ip, SIZE_SERVER_IP);

strcpy(myport, port);

What happens here?

It appears that servIP is allocated immediately after myport in memory.

 

[Tom St Denis]

What happens here?

It appears that servIP is allocated immediately after myport in memory.

Also the correct usage of strncpy [sadly] is this

memset(dest, 0, sizeof dest);
strncpy(dest, src, sizeof(dest) - 1);

Which is really F'ing stupid IMHO. A call to strncpy(dest, src,
sizeof dest) should guarantee that dest is NUL terminated...

You can optimize it a bit to

dest[sizeof(dest) - 1] = 0;
strncpy(dest, src, sizeof(dest) - 1);

But I find it's usually better for sanity sake to just zero the entire
buffer.

Tom

N.B. The man page for strncpy in Ubuntu has a typo ... the string is
nul terminated not null ...

 

[David Resnick]

What happens here?

It appears that servIP is allocated immediately after myport in memory.

Also the correct usage of strncpy [sadly] is this

memset(dest, 0, sizeof dest);
strncpy(dest, src, sizeof(dest) - 1);

Which is really F'ing stupid IMHO.  A call to strncpy(dest, src,
sizeof dest) should guarantee that dest is NUL terminated...

You can optimize it a bit to

dest[sizeof(dest) - 1] = 0;
strncpy(dest, src, sizeof(dest) - 1);

But I find it's usually better for sanity sake to just zero the entire
buffer.

Tom

N.B.  The man page for strncpy in Ubuntu has a typo ... the string is
nul terminated not null ...

strncpy should never be used IHMO. The lack of null termination is
crummy, and the memset it does as a side effect is awful if the max
size of the buffer is big and string copied is small. I can't
understand why you'd do a memset above, btw, rather than slap in the
null. I often see this in code -- memset a buffer then sprintf/
snprintf into it, makes no sense to me unless, say, the whole buffer
(regardless of length) is being sent over the wire or some such.

A better function to uses is strncat, it has much more useful
semantics.
dest[0] = '\0';
strncat(dest, src, sizeof(dest));

will handle the problem perfectly..

-David

 

[Ben Bacarisse]

What happens here?

It appears that servIP is allocated immediately after myport in memory.

Also the correct usage of strncpy [sadly] is this

memset(dest, 0, sizeof dest);
strncpy(dest, src, sizeof(dest) - 1);

Which is really F'ing stupid IMHO. A call to strncpy(dest, src,
sizeof dest) should guarantee that dest is NUL terminated...

You can optimize it a bit to

dest[sizeof(dest) - 1] = 0;
strncpy(dest, src, sizeof(dest) - 1);

But I find it's usually better for sanity sake to just zero the entire
buffer.

An alternative is strncat:

dest[0] = 0;
strncat(dest, src, sizeof(dest) - 1);

N.B. The man page for strncpy in Ubuntu has a typo ... the string is
nul terminated not null ...

Does the smiley mean you don't really mean this? I am never sure what a
smiley is intended to convey. It seems to be able express everything
from "I don't mean this at all -- it's a joke" to "I find this error
amusing".

 

[Rich Webb]

  strncpy(servIP, ip, SIZE_SERVER_IP);

  strcpy(myport, port);

What happens here?

It appears that servIP is allocated immediately after myport in memory.

Also the correct usage of strncpy [sadly] is this

memset(dest, 0, sizeof dest);
strncpy(dest, src, sizeof(dest) - 1);

Which is really F'ing stupid IMHO.  A call to strncpy(dest, src,
sizeof dest) should guarantee that dest is NUL terminated...

You can optimize it a bit to

dest[sizeof(dest) - 1] = 0;
strncpy(dest, src, sizeof(dest) - 1);

But I find it's usually better for sanity sake to just zero the entire
buffer.

Tom

N.B.  The man page for strncpy in Ubuntu has a typo ... the string is
nul terminated not null ...

strncpy should never be used IHMO. The lack of null termination is
crummy, and the memset it does as a side effect is awful if the max
size of the buffer is big and string copied is small. I can't
understand why you'd do a memset above, btw, rather than slap in the
null. I often see this in code -- memset a buffer then sprintf/
snprintf into it, makes no sense to me unless, say, the whole buffer
(regardless of length) is being sent over the wire or some such.

A better function to uses is strncat, it has much more useful
semantics.
dest[0] = '\0';
strncat(dest, src, sizeof(dest));

will handle the problem perfectly..

Or
strncpy(dest, src, LEN_DEST)[LEN_DEST - 1] = '\0';

Wouldn't sizeof(dest) be the size of a pointer to char?

 

[Tom St Denis]

Also the correct usage of strncpy [sadly] is this

memset(dest, 0, sizeof dest);
strncpy(dest, src, sizeof(dest) - 1);

Which is really F'ing stupid IMHO.  A call to strncpy(dest, src,
sizeof dest) should guarantee that dest is NUL terminated...

You can optimize it a bit to

dest[sizeof(dest) - 1] = 0;
strncpy(dest, src, sizeof(dest) - 1);

But I find it's usually better for sanity sake to just zero the entire
buffer.

N.B.  The man page for strncpy in Ubuntu has a typo ... the string is
nul terminated not null ...

strncpy should never be used IHMO.  The lack of null termination is
crummy, and the memset it does as a side effect is awful if the max
size of the buffer is big and string copied is small.  I can't
understand why you'd do a memset above, btw, rather than slap in the
null.  I often see this in code -- memset a buffer then sprintf/
snprintf into it, makes no sense to me unless, say, the whole buffer
(regardless of length) is being sent over the wire or some such.

Often it's nice to have known quantities in buffers. Like when I'm
writing a simple parser [to say separate out comma separated values]
the temp buffer I accumulate into is always zero'ed. It takes an
extra 10 microseconds and if I happen to be off-by-one somewhere at
least the buffer has NULs in it.

A better function to uses is strncat, it has much more useful
semantics.
dest[0] = '\0';
strncat(dest, src, sizeof(dest));

will handle the problem perfectly..

A *better* solution is to have a strncpy that guarantees a NUL in the
first n-bytes of the string...

Tom

 

[Tom St Denis]

An alternative is strncat:

  dest[0] = 0;
  strncat(dest, src, sizeof(dest) - 1);

Thing is even strncat is fubared.... from the man page

--
If src contains n or more characters, strncat() writes n+1
characters
to dest (n from src plus the terminating null byte). Therefore,
the
size of dest must be at least strlen(dest)+n+1.
--

So as you correctly pointed out (and David Resnick got wrong) you need
to -1 the sizeof of the buffer.

Does the smiley mean you don't really mean this?  I am never sure what a
smiley is intended to convey.  It seems to be able express everything
from "I don't mean this at all -- it's a joke" to "I find this error
amusing".

It's amusing. It's not the end of the world as I know what they
meant...

Tom

 

[David Resnick]

So as you correctly pointed out (and David Resnick got wrong) you need
to -1 the sizeof of the buffer.

Yep, oops. Don't use either much at this point. Still thing strncpy
is awful tho.

-David

 

[Tom St Denis]

Yep, oops.  Don't use either much at this point.  Still thing strncpy
is awful tho.

Since you still have to use -1 on strncat how is it any different than

dest[sizeof(dest)-1] = 0;
strncpy(dest, src, ...);

It's basically the same code at that point.

It'd be nice if all strn*() functions guaranteed that within n-bytes
of the dest there is a NUL and just truncate as required. But that
would require a bit of forethought on the standard C library designers
part.

Tom

 

[David Resnick]

  strncpy(servIP, ip, SIZE_SERVER_IP);
  strcpy(myport, port);
What happens here?
It appears that servIP is allocated immediately after myport in memory.
Also the correct usage of strncpy [sadly] is this
memset(dest, 0, sizeof dest);
strncpy(dest, src, sizeof(dest) - 1);
Which is really F'ing stupid IMHO.  A call to strncpy(dest, src,
sizeof dest) should guarantee that dest is NUL terminated...
You can optimize it a bit to
dest[sizeof(dest) - 1] = 0;
strncpy(dest, src, sizeof(dest) - 1);
But I find it's usually better for sanity sake to just zero the entire
buffer.
Tom
N.B.  The man page for strncpy in Ubuntu has a typo ... the string is
nul terminated not null ...

strncpy should never be used IHMO.  The lack of null termination is
crummy, and the memset it does as a side effect is awful if the max
size of the buffer is big and string copied is small.  I can't
understand why you'd do a memset above, btw, rather than slap in the
null.  I often see this in code -- memset a buffer then sprintf/
snprintf into it, makes no sense to me unless, say, the whole buffer
(regardless of length) is being sent over the wire or some such.

Often it's nice to have known quantities in buffers.  Like when I'm
writing a simple parser [to say separate out comma separated values]
the temp buffer I accumulate into is always zero'ed.  It takes an
extra 10 microseconds and if I happen to be off-by-one somewhere at
least the buffer has NULs in it.

Rather just write it correctly and check (though my off by one above
shows that doesn't always happen). If I'm hand writing to a buffer
MAYBE you have a point, if I'm using functions guaranteed to properly
terminate the string (sprintf/snprintf/strcat/strncat/strcpy/etc) I
don't get it. And sometimes the buffer is big and then it really
offends my sensibilities.

Consider the following code:
#define MAX_DEBUG 4096
char buf[MAX_DEBUG];
memset(buf,'\0', sizeof(buf));
strncpy(buf, "Checkpoint Charlie reached", sizeof(buf);

Bleh. A 4096 byte memset that serves no purpose. Then another > 4000
byte memset inside the strncpy just in case. Suspenders and belt,
where neither is needed and they cost you something to boot. Doing
needless work that doesn't make the code in any simpler/better seems
wrong to me...

-David

 

[David Resnick]

Yep, oops.  Don't use either much at this point.  Still thing strncpy
is awful tho.

Since you still have to use -1 on strncat how is it any different than

dest[sizeof(dest)-1] = 0;
strncpy(dest, src, ...);

It's basically the same code at that point.

It'd be nice if all strn*() functions guaranteed that within n-bytes
of the dest there is a NUL and just truncate as required.  But that
would require a bit of forethought on the standard C library designers
part.

Tom

Again, because strncpy has a design second flaw for general purpose
string usage, that it internally does a stupid memset that can be
QUITE costly if the target buffer is big and the string to be copied
is small.

-David

 

[Tom St Denis]

Since you still have to use -1 on strncat how is it any different than

dest[sizeof(dest)-1] = 0;
strncpy(dest, src, ...);

It's basically the same code at that point.

It'd be nice if all strn*() functions guaranteed that within n-bytes
of the dest there is a NUL and just truncate as required.  But that
would require a bit of forethought on the standard C library designers
part.

Tom

Again, because strncpy has a design second flaw for general purpose
string usage, that it internally does a stupid memset that can be
QUITE costly if the target buffer is big and the string to be copied
is small.

In my current app I'm more worried about buffer overflows or corrupt
[uninitialized] strings than a few microseconds of performance.

But as I said you *could* just set the n-1'th byte to 0 and get the
same effect.

Tom

 

[David Resnick]

So as you correctly pointed out (and David Resnick got wrong) you need
to -1 the sizeof of the buffer.
Yep, oops.  Don't use either much at this point.  Still thing strncpy
is awful tho.
Since you still have to use -1 on strncat how is it any different than
dest[sizeof(dest)-1] = 0;
strncpy(dest, src, ...);
It's basically the same code at that point.
It'd be nice if all strn*() functions guaranteed that within n-bytes
of the dest there is a NUL and just truncate as required.  But that
would require a bit of forethought on the standard C library designers
part.
Tom

Again, because strncpy has a design second flaw for general purpose
string usage, that it internally does a stupid memset that can be
QUITE costly if the target buffer is big and the string to be copied
is small.

In my current app I'm more worried about buffer overflows or corrupt
[uninitialized] strings than a few microseconds of performance.

But as I said you *could* just set the n-1'th byte to 0 and get the
same effect.

Tom

I guess I somehow wasn't thinking about your current app (how could I
have been?), but rather was making a general point that the semantics
of strncpy make it unwise to use. I put it down there in the category
of functions like strtok that I avoid in any real program. But YMMV.

-David

 

[Tom St Denis]

So as you correctly pointed out (and David Resnick got wrong) you need
to -1 the sizeof of the buffer.
Yep, oops.  Don't use either much at this point.  Still thing strncpy
is awful tho.
Since you still have to use -1 on strncat how is it any different than
dest[sizeof(dest)-1] = 0;
strncpy(dest, src, ...);
It's basically the same code at that point.
It'd be nice if all strn*() functions guaranteed that within n-bytes
of the dest there is a NUL and just truncate as required.  But that
would require a bit of forethought on the standard C library designers
part.
Tom
Again, because strncpy has a design second flaw for general purpose
string usage, that it internally does a stupid memset that can be
QUITE costly if the target buffer is big and the string to be copied
is small.

In my current app I'm more worried about buffer overflows or corrupt
[uninitialized] strings than a few microseconds of performance.

But as I said you *could* just set the n-1'th byte to 0 and get the
same effect.

Tom

I guess I somehow wasn't thinking about your current app (how could I
have been?), but rather was making a general point that the semantics
of strncpy make it unwise to use.  I put it down there in the category
of functions like strtok that I avoid in any real program.  But YMMV.

My point though is if you just set the last byte and then use strncpy
it's no less problematic or performance hazardy than setting the first
byte and using strncat. And since for BOTH functions you need to pass
n-1 as the length, they're both a PITA to work with.

So the comment that strncat is better than strncpy is just false. And
the fact that in your offhand snippet you got it wrong shows that it's
not as simple as it seems [not trying to disparage you, just saying
it's clearly not something people get right off the top of their
heads].

Tom

 

[David Resnick]

So as you correctly pointed out (and David Resnick got wrong) you need
to -1 the sizeof of the buffer.
Yep, oops.  Don't use either much at this point.  Still thing strncpy
is awful tho.
Since you still have to use -1 on strncat how is it any different than
dest[sizeof(dest)-1] = 0;
strncpy(dest, src, ...);
It's basically the same code at that point.
It'd be nice if all strn*() functions guaranteed that within n-bytes
of the dest there is a NUL and just truncate as required.  But that
would require a bit of forethought on the standard C library designers
part.
Tom
Again, because strncpy has a design second flaw for general purpose
string usage, that it internally does a stupid memset that can be
QUITE costly if the target buffer is big and the string to be copied
is small.
In my current app I'm more worried about buffer overflows or corrupt
[uninitialized] strings than a few microseconds of performance.
But as I said you *could* just set the n-1'th byte to 0 and get the
same effect.
Tom

I guess I somehow wasn't thinking about your current app (how could I
have been?), but rather was making a general point that the semantics
of strncpy make it unwise to use.  I put it down there in the category
of functions like strtok that I avoid in any real program.  But YMMV.

My point though is if you just set the last byte and then use strncpy
it's no less problematic or performance hazardy than setting the first
byte and using strncat.  And since for BOTH functions you need to pass
n-1 as the length, they're both a PITA to work with.

I don't get it. They have different performance. Again, consider the
following:

char buf[4096];

a) strncat
buf[0] = '\0';
strncat(buf, "a", sizeof(buf));

This will copy 2 bytes from the string "a", including the terminator.
All done.

b) strncpy
buf[sizeof(buf)-1] = '\0';
strncpy(buf, "a", sizeof(buf));

This will copy 2 bytes of the string "a". strncpy will then
*internally* do a 4094 byte memset as required by the C spec and
documented on the linux man page.

Mind you, to the end user the results are the same, if you don't mind
a 4094 byte memset that MOSTLY you don't care about and just are
looking at the string anyway.

-David

 

[David Resnick]

b) strncpy
buf[sizeof(buf)-1] = '\0';
strncpy(buf, "a", sizeof(buf));

Post in haste, repent at leisure. Need to reverse the order of those
two statements to guarantee termination or subtract one from size.
Moot in this case.

-David

 

[Ben Bacarisse]

An alternative is strncat:

  dest[0] = 0;
  strncat(dest, src, sizeof(dest) - 1);

Thing is even strncat is fubared.... from the man page

--
If src contains n or more characters, strncat() writes n+1
characters
to dest (n from src plus the terminating null byte). Therefore,
the
size of dest must be at least strlen(dest)+n+1.

I don't find that too much of a problem though the inconsistent use of
buffer sizes in various library functions is annoying. I proffered this
option simply because the dest[0] = 0 is easier than what is required
for strncpy.

It's amusing. It's not the end of the world as I know what they
meant...

Not sure what you mean now. My Ubuntu man page (for strncpy) has no
"nul" in it at all -- it looks fine to me.

 

[Tom St Denis]

b) strncpy
buf[sizeof(buf)-1] = '\0';
strncpy(buf, "a", sizeof(buf));

Post in haste, repent at leisure.  Need to reverse the order of those
two statements to guarantee termination or subtract one from size.
Moot in this case.

Ah fair enough. So strncpy is full of failure for MULTIPLE reasons.
That's impressive.

char *strncpy(char *dest, const char *src, size_t n)
{
char *tmp = dest;
while (*src && n-- > 1) {
*dest++ = *src++;
}
*dest = 0;
return tmp;
}

Is that really that hard?

Tom

 

[Tom St Denis]

An alternative is strncat:
  dest[0] = 0;
  strncat(dest, src, sizeof(dest) - 1);

Thing is even strncat is fubared.... from the man page

--
If src contains n or more characters, strncat() writes  n+1
characters
to  dest  (n  from src plus the terminating null byte).  Therefore,
the
size of dest must be at least strlen(dest)+n+1.
--

So as you correctly pointed out (and David Resnick got wrong) you need
to -1 the sizeof of the buffer.

I don't find that too much of a problem though the inconsistent use of
buffer sizes in various library functions is annoying.  I proffered this
option simply because the dest[0] = 0 is easier than what is required
for strncpy.

Yeah I missed the part where strncpy zeroes the rest of the buffer.
Whatever, in my current app it's not a problem. Correctness is more
important.

Not sure what you mean now.  My Ubuntu man page (for strncpy) has no
"nul" in it at all -- it looks fine to me.

The character '\0' is not a NULL it's NUL as in ASCII zero.

NULL in most programming contexts means a pointer, 'null' [lower case]
means an empty set. 'nul' means the zero char [or terminator].

Tom