Re: [LTP] lib: ujson: Update to ujson 1.0

[linuxtestproject.agent@gmail.com]

Hi Cyril,

On Mon Nov 9 17:32:22 2026 +0100, Cyril Hrubis <chrubis@suse.cz> wrote:
> lib: ujson: Update to ujson 1.0

> diff --git a/include/ujson_common.h b/include/ujson_common.h
> index 1eaf3fc72..ed31c090d 100644
> --- a/include/ujson_common.h
> +++ b/include/ujson_common.h
> @@ -8,12 +8,12 @@
>   * @brief Common JSON reader/writer definitions.
>   */
>  
> -#ifndef UJSON_COMMON_H__
> -#define UJSON_COMMON_H__
> +#ifndef UJSON_COMMON_H
> +#define UJSON_COMMON_H
>  
>  /** @brief Maximal error message length. */
>  #define UJSON_ERR_MAX 128
> -/** @brief Maximal id string length including terminating null element. */
> +/** @brief Maximal id string lenght including terminating null element. */

This hunk (along with the comment changes in include/ujson_reader.h,
include/ujson_utf.h, and include/ujson_writer.h) reverts the spelling fixes
from commit f6933a16f304 ("include: Fix API comment spelling and style"),
reintroducing "lenght", "intializer", "An user", "An union", and
"an unicode".

> diff --git a/include/ujson_reader.h b/include/ujson_reader.h
> index 3722fffea..273fe624a 100644
> --- a/include/ujson_reader.h
> +++ b/include/ujson_reader.h
> @@ -540,4 +540,4 @@ static inline int ujson_reader_consumed(ujson_reader *self)
>  	return self->off >= self->len;
>  }
>  
> -#endif /* UJSON_H__ */
> +#endif /* UJSON_H */

The closing guard comment should be /* UJSON_READER_H */ to match the
#ifndef UJSON_READER_H guard at the top of the file.

> diff --git a/libs/ujson/ujson_common.c b/libs/ujson/ujson_common.c
> index 639955229..c9cada9a9 100644
> --- a/libs/ujson/ujson_common.c
> +++ b/libs/ujson/ujson_common.c
> @@ -35,3 +35,4 @@ const char *ujson_type_name(enum ujson_type type)
>  		return "invalid";
>  	}
>  }
> +

This adds a trailing blank line at EOF (also in libs/ujson/ujson_writer.c).

> diff --git a/libs/ujson/ujson_reader.c b/libs/ujson/ujson_reader.c
> index 9f86f25b7..964878b51 100644
> --- a/libs/ujson/ujson_reader.c
> +++ b/libs/ujson/ujson_reader.c
> @@ -229,31 +258,15 @@ static int copy_str(ujson_reader *buf, char *str, size_t len)
>  
>  static int copy_id_str(ujson_reader *buf, char *str, size_t len)
>  {
> -	size_t pos = 0;
> -
>  	if (eatws(buf))
>  		goto err0;
>  
> -	if (!eatb(buf, '"'))
> +	if (peekb(buf) != '"')
>  		goto err0;
>  
> -	for (;;) {
> -		if (buf_empty(buf)) {
> -			ujson_err(buf, "Unterminated ID string");
> -			return 1;
> -		}
> -
> -		if (eatb(buf, '"')) {
> -			str[pos] = 0;
> -			break;
> -		}
> -
> -		if (pos >= len-1) {
> -			ujson_err(buf, "ID string too long");
> -			return 1;
> -		}
> -
> -		str[pos++] = getb(buf);
> +	if (copy_str(buf, str, len)) {
> +		ujson_err(buf, "Unterminated ID string");
> +		return 1;
>  	}

Every error path in copy_str() already sets a specific error via
ujson_err() (such as "String buffer too short!" when the ID exceeds
UJSON_ID_MAX, or "Invalid escape \\%c"). Calling ujson_err(buf,
"Unterminated ID string") here overwrites that error message even when the
ID string is properly terminated; drop the extra ujson_err() call.

> @@ -299,17 +312,28 @@ static int get_int(ujson_reader *buf, struct ujson_val *res)
>  		return 1;
>  	}
>  
> +	unsigned long max = LONG_MAX;
> +
> +	if (sign < 0)
> +		max++;
> +
>  	while (is_digit(peekb(buf))) {
> -		val *= 10;
> -		val += getb(buf) - '0';
> -		//TODO: overflow?
> +		unsigned int d = getb(buf) - '0';
> +
> +		if (val > (max - d) / 10) {
> +			ujson_err(buf, "Integer overflow");
> +			return 1;
> +		}
> +
> +		val = val * 10 + d;
>  	}
>  
>  	if (sign < 0)
> -		val = -val;
> +		res->val_int = -(long)val;
> +	else
> +		res->val_int = (long)val;

When parsing LONG_MIN, val is (unsigned long)LONG_MAX + 1, so -(long)val
casts to LONG_MIN first and then negates LONG_MIN in signed long
arithmetic, which is signed integer overflow UB. Negate before casting
with (long)-val to perform the negation in unsigned long arithmetic.

> diff --git a/libs/ujson/ujson_writer.c b/libs/ujson/ujson_writer.c
> index c86a3b2af..214e20846 100644
> --- a/libs/ujson/ujson_writer.c
> +++ b/libs/ujson/ujson_writer.c
> @@ -389,15 +396,25 @@ struct json_writer_file {
>  
>  static int out_writer_file_write(ujson_writer *self, int fd, const char *buf, ssize_t buf_len)
>  {
> +	if (!buf_len)
> +		return 0;
> +
>  	do {
>  		ssize_t ret = write(fd, buf, buf_len);
> -		if (ret <= 0) {
> +		if (ret < 0) {
>  			err(self, "Failed to write to a file");
>  			return 1;
>  		}
>  
> +		if (ret == 0) {
> +			err(self, "The write() syscall returned 0");
> +			errno = EAGAIN;
> +			return 1;
> +		}
> +
>  		if (ret > buf_len) {
>  			err(self, "Wrote more bytes than requested?!");
> +			errno = EIO;
>  			return 1;
>  		}
>  
>  		buf_len -= ret;
>  	} while (buf_len);

If write() completes a partial write (0 < ret < buf_len), buf is not
advanced before the next iteration, so the loop writes the start of buf
again instead of the remaining bytes. Advance buf += ret alongside
buf_len -= ret.

Verdict - Needs revision

Pre-existing issues:
- In libs/ujson/ujson_writer.c, ujson_writer_file_open() checks
  if (!writer_file->fd) instead of if (writer_file->fd < 0), which fails
  to detect open() returning -1 and treats valid fd 0 as an error.

---
Note:

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp