Re: [PATCH] Btrfs-progs use safe string manipulation functions

[Goffredo Baroncelli <kreijack@libero.it>]

On 02/10/2011 02:29 PM, Eduardo Silva wrote:
> On Thu, 2011-02-10 at 12:39 +0100, Olaf van der Spek wrote:
>> On Thu, Feb 10, 2011 at 12:37 PM, Jeremy Sanders
>> <jeremy@jeremysanders.net> wrote:
>>> Olaf van der Spek wrote:
>>>
>>>> On Thu, Feb 10, 2011 at 12:08 PM, Thomas Bellman <bellman@nsc.liu.se>
>>>> wrote:
>>>>> strncpy(args.name, source, BTRFS_PATH_NAME_MAX);
>>>>> args.name[BTRFS_PATH_NAME_MAX] = '\0';
>>>>
>>>> That's silly. Isn't there a sane safe variant of strcpy?
>>>
>>> There's strlcpy, but it's not in glibc because of possible truncation
>>> errors!
>>
>> Then use a private wrapper.
>>
> 
> Here's the new patch:
> 
> ----
> [PATCH] Add safe string manipulation functions
> 
> Deprecate direct use of strcpy(3)
> The following string manipulation function has been added:
> 
>    - string_copy() : wrapper of strcpy(3)
>    - string_ncopy(): wrapper of strncpy(3)
> 
> both function compose safe NULL terminated strings.
> ----
> 
> I check that the code most of the time raise an error if the path is too
> long, so the new wrappers should be ok...
> 
> best, 
> 
> Eduardo Silva
>  
[...]
+char *string_copy(char *dest, const char *src)
+{
+	if (!dest || !src) {
+		fprintf(stderr, "ERROR: invalid string_copy() parameters");
+		exit(EXIT_FAILURE);
+	}
+
+	memset(dest, '\0', sizeof(dest));

What is the purpose of the line above ?  sizeof(dest) is a const value
(typically 4 or 8) !

I agree with Olaf that string_copy() is usefulness.

I suggest you to improve the check of the string length before the copy
(not in the copy function), and raising an error when the length of the
string is too long.

Regards
G.Baroncelli