[PATCH] dive.c:merge_text(): Slighly better string merge logic

Dirk Hohndel dirk at hohndel.org
Fri Mar 8 08:55:27 PST 2013


"Lubomir I. Ivanov" <neolit123 at gmail.com> writes:

> From: "Lubomir I. Ivanov" <neolit123 at gmail.com>
>
> When a dive is deleted, we always check if it has 'location' text,
> for example. If so, it must have been allocated in memory
> previously and so we should free() it.
>
> When two dive texts are merged in merge_text(), instead of checking
> if a string is empty as well, we should only check if the pointer
> is NULL and then use the other string.
>
> Even if a string only contains a \0 character, we are still going
> to allocate 1 byte of memory for it using strdup(). Otherwise we
> also have to reflect this on our dive delete logic.
>
> Fixes #90
>
> Signed-off-by: Lubomir I. Ivanov <neolit123 at gmail.com>
> ---
>  dive.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/dive.c b/dive.c
> index 1b4c783..2f74a55 100644
> --- a/dive.c
> +++ b/dive.c
> @@ -907,9 +907,9 @@ static char *merge_text(const char *a, const char *b)
>  	char *res;
>  	if (!a && !b)
>  		return NULL;
> -	if (!a || !*a)
> +	if (!a)
>  		return strdup(b);
> -	if (!b || !*b)
> +	if (!b)
>  		return strdup(a);
>  	if (!strcmp(a,b))
>  		return strdup(a);

I don't agree with this patch.

Imagine that a="" and b="Great dive location"
The existing code returns a copy of "Great dive location" while your
code returns "() or (Great dive locations)".
That is not an improvement.

Can you explain in more detail what you are trying to fix?

/D


More information about the subsurface mailing list