Re: [PATCH v9 02/21] strbuf.c: add `strbuf_join_argv()`

2018-09-30 Thread Thomas Gummerer
On 09/26, Paul-Sebastian Ungureanu wrote:
> Implement `strbuf_join_argv()` to join arguments
> into a strbuf.
> 
> Signed-off-by: Paul-Sebastian Ungureanu 
> ---
>  strbuf.c | 15 +++
>  strbuf.h |  7 +++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/strbuf.c b/strbuf.c
> index 64041c3c24..3eb431b2b0 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -259,6 +259,21 @@ void strbuf_addbuf(struct strbuf *sb, const struct 
> strbuf *sb2)
>   strbuf_setlen(sb, sb->len + sb2->len);
>  }
>  
> +const char *strbuf_join_argv(struct strbuf *buf,
> +  int argc, const char **argv, char delim)
> +{
> + if (!argc)
> + return buf->buf;
> +
> + strbuf_addstr(buf, *argv);
> + while (--argc) {
> + strbuf_addch(buf, delim);
> + strbuf_addstr(buf, *(++argv));
> + }
> +
> + return buf->buf;

Why are we returning buf-buf here?  The strbuf is modified by the
function, so the caller can just use buf->buf directly if they want
to.  Is there something I'm missing?

> +}
> +
>  void strbuf_addchars(struct strbuf *sb, int c, size_t n)
>  {
>   strbuf_grow(sb, n);
> diff --git a/strbuf.h b/strbuf.h
> index 60a35aef16..7ed859bb8a 100644
> --- a/strbuf.h
> +++ b/strbuf.h
> @@ -284,6 +284,13 @@ static inline void strbuf_addstr(struct strbuf *sb, 
> const char *s)
>   */
>  extern void strbuf_addbuf(struct strbuf *sb, const struct strbuf *sb2);
>  
> +

stray newline? We usually only have one blank line between functions.

> +/**
> + *
> + */

Forgot to write some documentation here? :)

> +extern const char *strbuf_join_argv(struct strbuf *buf, int argc,
> + const char **argv, char delim);
> +
>  /**
>   * This function can be used to expand a format string containing
>   * placeholders. To that end, it parses the string and calls the specified
> -- 
> 2.19.0.rc0.23.g1fb9f40d88
> 


[PATCH v9 02/21] strbuf.c: add `strbuf_join_argv()`

2018-09-25 Thread Paul-Sebastian Ungureanu
Implement `strbuf_join_argv()` to join arguments
into a strbuf.

Signed-off-by: Paul-Sebastian Ungureanu 
---
 strbuf.c | 15 +++
 strbuf.h |  7 +++
 2 files changed, 22 insertions(+)

diff --git a/strbuf.c b/strbuf.c
index 64041c3c24..3eb431b2b0 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -259,6 +259,21 @@ void strbuf_addbuf(struct strbuf *sb, const struct strbuf 
*sb2)
strbuf_setlen(sb, sb->len + sb2->len);
 }
 
+const char *strbuf_join_argv(struct strbuf *buf,
+int argc, const char **argv, char delim)
+{
+   if (!argc)
+   return buf->buf;
+
+   strbuf_addstr(buf, *argv);
+   while (--argc) {
+   strbuf_addch(buf, delim);
+   strbuf_addstr(buf, *(++argv));
+   }
+
+   return buf->buf;
+}
+
 void strbuf_addchars(struct strbuf *sb, int c, size_t n)
 {
strbuf_grow(sb, n);
diff --git a/strbuf.h b/strbuf.h
index 60a35aef16..7ed859bb8a 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -284,6 +284,13 @@ static inline void strbuf_addstr(struct strbuf *sb, const 
char *s)
  */
 extern void strbuf_addbuf(struct strbuf *sb, const struct strbuf *sb2);
 
+
+/**
+ *
+ */
+extern const char *strbuf_join_argv(struct strbuf *buf, int argc,
+   const char **argv, char delim);
+
 /**
  * This function can be used to expand a format string containing
  * placeholders. To that end, it parses the string and calls the specified
-- 
2.19.0.rc0.23.g1fb9f40d88