[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Introduce virStrncpy.



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 veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]