patterncMinor
Substring function in C
Viewed 0 times
substringfunctionstackoverflow
Problem
I am trying to implement substring in C. How efficient and standard is this code? I have a hard time understanding how C functions are usually meant to work, like how
printf() returns an int or size_t.size_t substring(char *destination, char const *string, int start, int len) {
// writes a substring to the destinaton starting at start index of string until end stepping by step
// follows the same start at 0 off by one on the end format as Java substring method
int substringLength = len;
int stringLength = strlen(string);
if (start > stringLength || len > stringLength || start = len) {
fputs("start must be start < len", stderr);
return -1;
}
memcpy(destination, &string[start], substringLength);
if (destination[stringLength] != '\x0') {
destination[stringLength] = '\x0';
}
return substringLength;
}Solution
Argument confusion
If you were trying to replicate the java substring function, you should have had a
Bug 1
This line doesn't correctly check for the case of copying off the end of the source string:
It needs to be:
Edit: If you want to check for integer overflow, you can also throw in
Bug 2
This check is unnecessary and can return an error incorrectly:
For example, if the string has 10 bytes,
Bug 3
When you terminate the destination string, you use the wrong index. It should be
should be:
Also,
Unnecessary local variable
The variable
Return value
The java substring function returns the string. Your function returns the length but I don't think that this will be useful to the caller. The reason you might want to return a string is so you can chain calls like this:
If you were trying to replicate the java substring function, you should have had a
start and end argument. As it is, you have a start and length argument, but you didn't handle the length argument correctly.Bug 1
This line doesn't correctly check for the case of copying off the end of the source string:
if (start > stringLength || len > stringLength || start < 0 || len < 0) {It needs to be:
if (start + len > stringLength || start < 0 || len < 0) {Edit: If you want to check for integer overflow, you can also throw in
|| start + len < 0, and use size_t everywhere instead of int.Bug 2
This check is unnecessary and can return an error incorrectly:
if (start >= len) {
fputs("start must be start < len", stderr);
return -1;
}For example, if the string has 10 bytes,
start is 8 and len is 1, this check will return an error.Bug 3
When you terminate the destination string, you use the wrong index. It should be
len, not stringLength. Also, I'm not sure why you do a check before you write to it:if (destination[stringLength] != '\x0') {
destination[stringLength] = '\x0';
}should be:
destination[len] = '\0';Also,
'\0' is the standard way to write the null character. I've never seen it written as '\x0' before.Unnecessary local variable
The variable
substringLength is unnecessary because it is always the same as len. You can just use len instead.Return value
The java substring function returns the string. Your function returns the length but I don't think that this will be useful to the caller. The reason you might want to return a string is so you can chain calls like this:
strcat(substring(buf, path, dir_start, dir_length), basename);Code Snippets
if (start > stringLength || len > stringLength || start < 0 || len < 0) {if (start + len > stringLength || start < 0 || len < 0) {if (start >= len) {
fputs("start must be start < len", stderr);
return -1;
}if (destination[stringLength] != '\x0') {
destination[stringLength] = '\x0';
}destination[len] = '\0';Context
StackExchange Code Review Q#112688, answer score: 3
Revisions (0)
No revisions yet.