[patch v1] stdio: Add more setvbuf tests

Joseph Myers josmyers@redhat.com
Thu Mar 27 18:40:00 GMT 2025


On Mon, 24 Mar 2025, DJ Delorie wrote:

> +#include <dlfcn.h>

What's <dlfcn.h> used for in this test?

> +/* Dear future developer: If you are reading this, you are likely
> +   trying to change or understand this test.  In that case, these
> +   debug/dump macros will be helpful.  */
> +#if 0
> +#define debug printf ("\033[3%dm%s:%d\033[0m\n", \

Note the glibc style to use spaces after "#" for preprocessor directives 
inside #if, so "# define" here.

> +#else
> +#define debug
> +#define dumpfp(FP)

Likewise.

> +void
> +prepare_test_data (void)
> +{
> +  buffer = (char *) malloc (bufsize);

I'd expect a check for allocation failure (possibly through use of 
xmalloc).  Maybe the code works even if allocation fails, but calling 
setvbuf with a NULL buffer probably isn't testing the intended paths.

> +	if (mknod (test_file_name, S_IFIFO | 0600, 0) != 0)
> +	  FAIL_EXIT1 ("mknod: %m");

This could use xmkfifo to avoid needing a local error check here.

> +  switch (s)
> +    {
> +    case test_stream_stdin:
> +      fp = fopen (fname, "r");
> +      break;

In this code, you're consistently not checking for a null fp until later:

> +  if (f == test_source_pseudo_terminal)
> +    {
> +      struct termios t;
> +      /* We disable the NL to CR-LF conversion so that we can compare
> +	 data without having to remove the extra CRs.  */
> +      if (tcgetattr (fileno (fp), &t) < 0)
> +	FAIL_EXIT1 ("tcgetattr failed: %m");
> +      t.c_oflag &= ~ONLCR;
> +      if (tcsetattr (fileno (fp), TCSANOW, &t) < 0)
> +	FAIL_EXIT1 ("tcsetattr failed: %m");
> +    }
> +
> +  TEST_VERIFY_EXIT (fp != NULL);

In particular, the above uses fileno (fp) before the check for not being 
NULL.

If interfaces such as xfopen can be used above, that would be the simplest 
way to get early-enough error checks.

> +	      if (s <= test_stream_stderr)
> +		{
> +		  rv += recurse (fp, f, s, c);
> +		}
> +	      else
> +		{
> +		  rv += do_second_part (fp, f, s, c);
> +		}

The normal style in glibc would be not to have braces around a single 
expression statement like this.

-- 
Joseph S. Myers
josmyers@redhat.com



More information about the Libc-alpha mailing list