×
INTELLIGENT WORK FORUMS
FOR COMPUTER PROFESSIONALS

Log In

Come Join Us!

Are you a
Computer / IT professional?
Join Tek-Tips Forums!
  • Talk With Other Members
  • Be Notified Of Responses
    To Your Posts
  • Keyword Search
  • One-Click Access To Your
    Favorite Forums
  • Automated Signatures
    On Your Posts
  • Best Of All, It's Free!
  • Students Click Here

*Tek-Tips's functionality depends on members receiving e-mail. By joining you are opting in to receive e-mail.

Posting Guidelines

Promoting, selling, recruiting, coursework and thesis posting is forbidden.

Students Click Here

Jobs

strcpy and strncpy safety stuff

strcpy and strncpy safety stuff

strcpy and strncpy safety stuff

(OP)
So, the ability to overflow with strcpy() is fairly obvious, and I see how strncpy() isn't the best answer. It won't necessarily null-terminate, and it'll wastefully pad with NULLs. Also, these functions don't offer any kind of error reporting. What about this for a safe string copy? It copies only what's necessary, guarantees null-termination, and when an overflow is detected, returns how many characters past the buffer the source went - albeit a little oddly. It uses the negative portion of the integer range to express how many. The function returns:
positive on success
0 for null pointers for either source or destination
negative for overflow (the absolute value of which is how many bytes over)

CODE --> C

#include <string.h>

int strncpy0(char *dst,const char *src,int max)
{
	int i;
	size_t j;

	if(dst==NULL || src==NULL)
		return 0;

	for(i=0;(dst[i]=src[i])!='\0' && i<max; i++)
		;

	if(i>=max){
		dst[max-1]='\0';
		j=strlen(src);
		return i-j-1;
	}

	return i;
} 

RE: strcpy and strncpy safety stuff

C evaluation is from left to right. You will get an array OOB before i < max. The loop termination condition should be

CODE

for(i=0;i < max && (dst[i]=src[i])!='\0'; i++) 

RE: strcpy and strncpy safety stuff

It's defined in the C standard that functions return zero for success. This gives you more values to spell out the specific error that occurred. Functions often have one way to succeed, and many ways to fail.

Personally, I would use...

0 = Success.
-1 = Failure (NULL ptr).
>0 = Positive value for number of characters not copied.

This lets you use the positive number returned to know how many you still need to copy without having to flip the sign. That makes the return value a little more useful. Just use it.

I also agree with xwb's comment.

Last, you have a couple issues if this gets called with 'max' being 0 (zero). Your original code would still copy one character. xwb's code fixes that, but you would still end up trying to write a NULL byte at dst[-1] which can cause all kinds of unintended weirdness. Also, if called with 'max' being a negative number, it will also get messy. Maybe return a success immediately if max==0, and return negative error code if max<0.

RE: strcpy and strncpy safety stuff

Sam - strncpy returns the destination string. I think the op is just following that.

RE: strcpy and strncpy safety stuff

(OP)

Quote (xwb)

C evaluation is from left to right. You will get an array OOB before i < max...
D'oh. Thank you for catching that.

Quote (SamBones)

It's defined in the C standard that functions return zero for success...
Oh? I thought it was only a requirement for main(). I don't have a copy of the standard. That's just what I heard.

RE: strcpy and strncpy safety stuff

Yeah, it's not a hard and fast requirement as it's pretty easy to find exceptions. The basic idea behind it is that most functions have one way to succeed, and many ways to fail, so 0 for success and non-zero for any failure fits that. The problem I have with it is readability. A successfull zero is a boolean FALSE. You have to negate/not the return value to use it as a boolean value...

CODE

if ( ! some_function() )
    // It succeeded
else
    // It failed 

I do like what you're proposing, but I do like negative for errors and positive for count of leftovers. That gives you use cases like this...

CODE

rv = strncpysafe(destination, source, strlen(source));

if(rv < 0) return(FAILURE);

if(rv) {
    printf("Leftovers: %s\n", &source[rv]);
    // Copy more, copy somewhere else, log it, whatever
    // It's nice knowing how much didn't make it
    } 

RE: strcpy and strncpy safety stuff

(OP)

Quote (SamBones)

The problem I have with it is readability. A successfull zero is a boolean FALSE...
Exactly. It drives me nuts to see:

CODE --> C

if(!strcmp(a,b)) 
Even the C FAQ suggests against it (17.3), so I tend to base my return values on what feels the most intuitive.

RE: strcpy and strncpy safety stuff

(OP)
Ok. Here's the revision:

CODE --> C

int strncpy0(char *dst,const char *src,int max)
{
	int i;
	size_t j;

	if(dst==NULL || src==NULL || max<1)
		return -1;

	for(i=0;i<max && (dst[i]=src[i])!='\0'; i++)
		;

	if(i>=max){
		dst[max-1]='\0';
		j=strlen(src);
		return j-i+1;
	}

	return 0;
} 
It returns:
-1 on bad input
0 on success
positive for buffer overflow (positive value being the number of bytes over).

RE: strcpy and strncpy safety stuff

(OP)
Now that I think about it, seeing as the whole negative range is freed-up, I might as well take advantage of that and add greater specificity to the return values:

CODE --> C

int strncpy0(char *dst,const char *src,int max)
{
	int i;
	size_t j;

	if(dst==NULL)
		return -1;
	if(src==NULL)
		return -2;
	if(max<1)
		return -3;

	for(i=0;i<max && (dst[i]=src[i])!='\0'; i++)
		;

	if(i>=max){
		dst[max-1]='\0';
		j=strlen(src);
		return j-i+1;
	}

	return 0;
} 
So now it returns:
-1 on bad dst
-2 on bad src
-3 on bad max
0 on success
>0 on attempted buffer overflow

RE: strcpy and strncpy safety stuff

I like it! bigsmile

My only remaining heartburn is the name. I like descriptive names since I'm old and forgetful. I would prefer something like strncpysafe(). But it's your function. You can name it as you wish. bigsmile

RE: strcpy and strncpy safety stuff

(OP)
I'm not in love with the name. I'm open to others. thumbsup

Red Flag This Post

Please let us know here why this post is inappropriate. Reasons such as off-topic, duplicates, flames, illegal, vulgar, or students posting their homework.

Red Flag Submitted

Thank you for helping keep Tek-Tips Forums free from inappropriate posts.
The Tek-Tips staff will check this out and take appropriate action.

Reply To This Thread

Posting in the Tek-Tips forums is a member-only feature.

Click Here to join Tek-Tips and talk with other members! Already a Member? Login

Close Box

Join Tek-Tips® Today!

Join your peers on the Internet's largest technical computer professional community.
It's easy to join and it's free.

Here's Why Members Love Tek-Tips Forums:

Register now while it's still free!

Already a member? Close this window and log in.

Join Us             Close