On Thu, 2007-08-30 at 21:09 -0700, Linus Torvalds wrote:
>
> On Fri, 31 Aug 2007, Timo Sirainen wrote:
> > >
> > > Perhaps because your patch was using a totally nonstandard and slow
> > > interface, and had nasty string declaration issues, as people even pointed
> > > out to you.
> >
> > Slow?
>
> Having a string library, and then implementing "str_append()" with a
> strlen() sounds pretty disgusting to me.
>
> Gcc could have optimized the strlen() away for constant string arguments,
> but since you made the thing out-of-line, it can't do that any more.
>
> So yes, I bet there are faster string libraries out there.
Oh, well that's easy to fix. But I don't think the speed matters much in
string manipulation, it's usually not done in performance critical
paths.
> > The code should be easy to verify to be secure, and with some kind of a safe
> > string API it's a lot easier than trying to figure out corner cases where
> > strcpy() calls break.
>
> I actually looked at the patches, and the "stringbuf()" thing was just too
> ugly to live. It was also nonportable, in that you use the reserved
> namespace (which we do extensively in the kernel, but that's an
> "embdedded" application that doesn't use system header files).
>
> So the API was anything but "safe".
Some of those were fixed in the patch I sent at the beginning of this
thread. But since you asked, attached is yet another version of it. As
far as I know it's fully C99 compatible. Would be C89 but there's that
va_copy() call.
I guess it could still be optimized more, but at least strlen() is now
in a macro. :)
Or if you don't like the STR_STATIC() macro, another way would be:
#define STR_STATIC_INIT(buf) { buf, sizeof(buf), 0, }
char static_buf[1024];
struct string str = STR_STATIC_INIT(static_buf);
str_append(&str, "hello world");
--- /dev/null 2007-07-12 03:40:41.165212167 +0300
+++ str.h 2007-08-31 07:35:04.000000000 +0300
@@ -0,0 +1,33 @@
+#ifndef STR_H
+#define STR_H
+
+struct string {
+ char *buf;
+
+ unsigned int alloc_size;
+ unsigned int len;
+
+ unsigned int malloced:1;
+ unsigned int overflowed:1;
+};
+
+#define STR_STATIC(name, size) \
+ struct { \
+ struct string str; \
+ char string_buf[(size) + 1]; \
+ } name = { { (name).string_buf, sizeof((name).string_buf), 0, } }
+
+extern struct string *str_alloc(unsigned int initial_size);
+extern void str_free(struct string **str);
+
+extern void str_append_n(struct string *str, const char *cstr,
+ unsigned int len);
+extern void str_printfa(struct string *str, const char *fmt, ...)
+ __attribute__((format (printf, 2, 3)));
+extern void str_truncate(struct string *str, unsigned int len);
+
+#define str_append(str, cstr) str_append_n(str, cstr, strlen(cstr))
+#define str_c(str) ((str)->buf)
+#define str_len(str) ((str)->len)
+
+#endif
--- /dev/null 2007-07-12 03:40:41.165212167 +0300
+++ str.c 2007-08-31 07:36:56.000000000 +0300
@@ -0,0 +1,85 @@
+#include "str.h"
+#include "git-compat-util.h"
+
+struct string *str_alloc(unsigned int initial_size)
+{
+ struct string *str;
+
+ str = xcalloc(sizeof(*str), 1);
+ str->alloc_size = initial_size + 1;
+ str->buf = xmalloc(str->alloc_size);
+ str->buf[0] = '\0';
+ return str;
+}
+
+void str_free(struct string **_str)
+{
+ struct string *str = *_str;
+
+ if (str->malloced)
+ free(str->buf);
+ str->buf = NULL;
+ free(str);
+
+ *_str = NULL;
+}
+
+static void str_grow_if_needed(struct string *str, unsigned int *len)
+{
+ unsigned int avail = str->alloc_size - str->len;
+
+ if (*len >= avail) {
+ if (!str->malloced) {
+ /* static buffer size */
+ *len = avail;
+ str->overflowed = 1;
+ } else {
+ str->alloc_size = (str->len + *len) * 2;
+ str->buf = xrealloc(str->buf, str->alloc_size);
+ }
+ }
+}
+
+void str_append_n(struct string *str, const char *cstr, unsigned int len)
+{
+ str_grow_if_needed(str, &len);
+ memcpy(str->buf + str->len, cstr, len);
+ str->len += len;
+ str->buf[str->len] = '\0';
+}
+
+void str_printfa(struct string *str, const char *fmt, ...)
+{
+ unsigned int len, avail = str->alloc_size - str->len;
+ va_list va, va2;
+ int ret;
+
+ va_start(va, fmt);
+ va_copy(va2, va);
+ ret = vsnprintf(str->buf + str->len, avail, fmt, va);
+ assert(ret >= 0);
+
+ if ((unsigned int)ret >= avail) {
+ if (!str->malloced) {
+ str->overflowed = 1;
+ ret = avail == 0 ? 0 : avail-1;
+ } else {
+ len = ret;
+ str_grow_if_needed(str, &len);
+ avail = str->alloc_size - str->len;
+
+ ret = vsnprintf(str->buf + str->len, avail, fmt, va2);
+ assert(ret >= 0 && (unsigned int)ret < avail);
+ }
+ }
+ str->len += ret;
+ va_end(va);
+}
+
+void str_truncate(struct string *str, unsigned int len)
+{
+ if (len >= str->alloc_size)
+ len = str->alloc_size - 1;
+ str->len = len;
+ str->buf[len] = '\0';
+}
Attachment:
signature.asc
Description: This is a digitally signed message part
