lib/string/: strerrno(): Use statement expression to perform lvalue conversion

[alejandro-colomar]

Compound literals are lvalues.  This means it's possible to take their
address.  That is, it would be possible (albeit nonsensical) to do

        &strerrno();

It is also possible to assign to them (albeit also nonsensical):

        strerrno() = NULL;

The statement expression performs lvalue conversion, which turns the
lvalue into an "rvalue", as expected, and disallows all those issues.

---

Revisions:

v2

• Use a statement expression. It's a non-standard extension, but it is safer, and produces better diagnostics.

$ git rd 
1:  79de0c814 ! 1:  885f4356f lib/string/: strerrno(): Use 'register' to forbid taking the address of the return value
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>
     
      ## Commit message ##
    -    lib/string/: strerrno(): Use 'register' to forbid taking the address of the return value
    +    lib/string/: strerrno(): Use statement expression to perform lvalue conversion
     
         Compound literals are lvalues.  This means it's possible to take their
         address.  That is, it would be possible (albeit nonsensical) to do
     
                 &strerrno();
     
    -    The 'register' storage-class specifier converts such code into
    -    a compiler error.
    +    It is also possible to assign to them (albeit also nonsensical):
    +
    +            strerrno() = NULL;
    +
    +    The statement expression performs lvalue conversion, which turns the
    +    lvalue into an "rvalue", as expected, and disallows all those issues.
     
         Signed-off-by: Alejandro Colomar <alx@kernel.org>
     
    @@ lib/string/strerrno.h
      
      // strerrno - string errno
     -#define strerrno()  ((const char *){strerror(errno)})
    -+#define strerrno()  ((register const char *){strerror(errno)})
    ++#define strerrno()  ({(const char *){strerror(errno)};})
      
      
      #endif  // include guard

[hallyn]

What is the concern here?

[alejandro-colomar]

What is the concern here?

Maybe this example shows the concern:

alx@devuan:~/tmp$ cat strerrno.c 
#include <errno.h>
#include <stddef.h>
#include <stdio.h>
#include <string.h>

#define strerrno1()  ((const char *){strerror(errno)})
#define strerrno2()  ({(const char *){strerror(errno)};})

int
main(void)
{
	if (strerrno1() = NULL)  // Oops, I meant == NULL
		return 1;

	if (strerrno2() = NULL)  // Oops, I meant == NULL
		return 2;

	printf("%s\n", &strerrno1());
	printf("%s\n", &strerrno2());
}

alx@devuan:~/tmp$ gcc strerrno.c 
strerrno.c: In function ‘main’:
strerrno.c:15:25: error: lvalue required as left operand of assignment
   15 |         if (strerrno2() = NULL)  // Oops, I meant == NULL
      |                         ^
strerrno.c:19:24: error: lvalue required as unary ‘&’ operand
   19 |         printf("%s\n", &strerrno2());
      |                        ^

With the first implementation, those trivial bugs are not caught by the compiler.
(Admittedly, with -Wall and -Wextra, the compiler might catch them for other reasons. Still, this is an obvious safety improvement, so I think it's good.)

With the second implementation (the one used in this PR), the compiler is able to diagnose them.

[alejandro-colomar]

A cast (alternative 3) would also serve the same purpose, but casts have their own problems (they silence almost every diagnostic), so I don't want to use them if we can avoid them.

alx@devuan:~/tmp$ cat strerrno.c 
#include <errno.h>
#include <stddef.h>
#include <stdio.h>
#include <string.h>

#define strerrno1()  ((const char *){strerror(errno)})
#define strerrno2()  ({(const char *){strerror(errno)};})
#define strerrno3()  ((const char *) strerror(errno))

int
main(void)
{
	if (strerrno1() = NULL)  // Oops, I meant == NULL
		return 1;
	if (strerrno2() = NULL)  // Oops, I meant == NULL
		return 2;
	if (strerrno3() = NULL)  // Oops, I meant == NULL
		return 3;

	printf("%s\n", &strerrno1());
	printf("%s\n", &strerrno2());
	printf("%s\n", &strerrno3());
}

alx@devuan:~/tmp$ gcc strerrno.c 
strerrno.c: In function ‘main’:
strerrno.c:15:25: error: lvalue required as left operand of assignment
   15 |         if (strerrno2() = NULL)  // Oops, I meant == NULL
      |                         ^
strerrno.c:17:25: error: lvalue required as left operand of assignment
   17 |         if (strerrno3() = NULL)  // Oops, I meant == NULL
      |                         ^
strerrno.c:21:24: error: lvalue required as unary ‘&’ operand
   21 |         printf("%s\n", &strerrno2());
      |                        ^
strerrno.c:22:24: error: lvalue required as unary ‘&’ operand
   22 |         printf("%s\n", &strerrno3());
      |                        ^

[hallyn]

Ok, that's what I thought, but that's purely defensive against our own future
mistakes. There's nothing that an attacker can do if we haven't fouled it up,
right?

Still might be worth doing, as I think there are no side effects to worry about.
But I wanted to make sure I was understanding the concern. Thanks.

[alejandro-colomar]

Ok, that's what I thought, but that's purely defensive against our own future mistakes. There's nothing that an attacker can do if we haven't fouled it up, right?

Right. There's nothing exploitable. It's purely defensive against our own future silly mistakes. :)

Still might be worth doing, as I think there are no side effects to worry about. But I wanted to make sure I was understanding the concern. Thanks.

You're welcome!