On Tue, Sep 22, 2009 at 04:16:51PM +0200, Chris Lalancette wrote:
Daniel Veillard wrote:
> On Tue, Sep 22, 2009 at 02:23:26PM +0200, Chris Lalancette wrote:
>> Add the virStrncpy function, which takes a dst string, source string,
>> the number of bytes to copy and the number of bytes available in the
>> dest string. If the source string is too large to fit into the
>> destination string, including the \0 byte, then no data is copied and
>> the function returns NULL. Otherwise, this function copies n bytes
>> from source into dst, including the \0, and returns a pointer to the
>> dst string. This function is intended to replace all unsafe uses
>> of strncpy in the code base, since strncpy does *not* guarantee that
>> the buffer terminates with a \0.
> [...]
>> diff --git a/src/util/util.h b/src/util/util.h
>> index f9715ab..2489f63 100644
>> --- a/src/util/util.h
>> +++ b/src/util/util.h
>> @@ -164,6 +164,10 @@ void virSkipSpaces(const char **str);
>> int virParseNumber(const char **str);
>> int virAsprintf(char **strp, const char *fmt, ...)
>> ATTRIBUTE_FMT_PRINTF(2, 3);
>> +char *virStrncpy(char *dest, const char *src, size_t n, size_t destbytes)
>> + ATTRIBUTE_RETURN_CHECK;
>> +#define virStrcpy(dest, src, destbytes) virStrncpy(dest, src, strlen(src),
destbytes)
>> +#define virStrcpyStatic(dest, src) virStrncpy(dest, src, strlen(src),
sizeof(dest))
>>
>> #define VIR_MAC_BUFLEN 6
>> #define VIR_MAC_PREFIX_BUFLEN 3
>
> I would just feel beter if we kept virStrcpy and virStrcpyStatic real
> functions. I prefer my macros names all uppercase and it's not like we
> will gain much using macros. Actually the compiler (if gcc) probably
> optimize much of this out, and the fact we are using a wrapper will
> mostly kill those. The point is cleaning things up, not optimizing :)
>
> ACK but I would feel a bit better with purely function entry points
> ... that can still be done later as a tiny patch
Unfortunately that's not possible in the case of virStrcpyStatic(). Because
virStrcpyStatic() does a sizeof(dest), you have to have the original char
foo[123], not a char *, to get something meaningful. That means that it has to
either be a macro or not exist at all. (I could be convinced of the latter, but
I found it convenient when converting a number of these locations)
I could make virStrcpy a small function, though.
Gahh, right ! I completely missed that :-\
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit
http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine
http://rpmfind.net/
http://veillard.com/ | virtualization library
http://libvirt.org/