Re: [PATCH v3 01/15] strbuf: introduce strbuf_split_str_omit_term()

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

 



Karthik Nayak <karthik.188@xxxxxxxxx> writes:

> The current implementation of 'strbuf_split_buf()' includes the
> terminator at the end of each strbuf post splitting. Add an option
> wherein we can drop the terminator if desired. In this context
> introduce a wrapper function 'strbuf_split_str_omit_term()' which
> splits a given string into strbufs without including the terminator.
>
> Helped-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx>
> Signed-off-by: Karthik Nayak <Karthik.188@xxxxxxxxx>
> ---
>  strbuf.c |  7 ++++---
>  strbuf.h | 25 ++++++++++++++++---------
>  2 files changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/strbuf.c b/strbuf.c
> index b552a13..b62508e 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -115,7 +115,7 @@ void strbuf_tolower(struct strbuf *sb)
>  }
>  
>  struct strbuf **strbuf_split_buf(const char *str, size_t slen,
> -				 int terminator, int max)
> +				 int terminator, int max, int omit_term)
>  {
>  	struct strbuf **ret = NULL;
>  	size_t nr = 0, alloc = 0;
> @@ -123,14 +123,15 @@ struct strbuf **strbuf_split_buf(const char *str, size_t slen,
>  
>  	while (slen) {
>  		int len = slen;
> +		const char *end = NULL;
>  		if (max <= 0 || nr + 1 < max) {
> -			const char *end = memchr(str, terminator, slen);
> +			end = memchr(str, terminator, slen);
>  			if (end)
>  				len = end - str + 1;
>  		}
>  		t = xmalloc(sizeof(struct strbuf));
>  		strbuf_init(t, len);
> -		strbuf_add(t, str, len);
> +		strbuf_add(t, str, len - !!end * !!omit_term);

You initialize with "len" but sometimes copy less than that, which
looks somewhat sloppy.

Maybe I am old-fashioned, but use of a multiplication when you do
not mean to numerically multiply but merely to perform a logical
operation made me go "Huh?".

Perhaps using another variable would make it easier to follow?
Either using a boolean that tells us that the terminating byte
is to be omitted, i.e.

	int len = slen;
        int omit = 0;
        if ( ... we are still splitting ... ) {
		const char *end = memchr(...);
                if (end) {
                	len = end - str + 1;
			omit = !!omit_term;
		}
	}
	strbuf_init(t, len - omit);
        strbuf_add(t, str, len - omit);

or an integer "copylen" that tells us how many bytes to copy, which
often is the same as "len" but sometimes different by 1 byte?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]