[5/6] tests: Overly elaborate compiler warning workaround

Submitted by Daniel Stone on Aug. 29, 2018, 6:17 a.m.

Details

Message ID 20180829061715.27776-6-daniels@collabora.com
State Accepted
Commit cb9a2557e11f294256943f5b4187940d7234820c
Headers show
Series "Minor test/scanner fixes" ( rev: 1 ) in Wayland (DEPRECATED)

Commit Message

Daniel Stone Aug. 29, 2018, 6:17 a.m.
Clang will rightly point out that example_sockaddr_un in socket-test
will get discarded from the compilation unit as it is completely unused.
Put in a couple of lines which of no value other than stopping Clang
from complaining.

Signed-off-by: Daniel Stone <daniels@collabora.com>
---
 tests/socket-test.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/tests/socket-test.c b/tests/socket-test.c
index e9705628..8d39edce 100644
--- a/tests/socket-test.c
+++ b/tests/socket-test.c
@@ -42,7 +42,7 @@ 
  * See `man 7 unix`.
  */
 
-static const struct sockaddr_un example_sockaddr_un;
+static struct sockaddr_un example_sockaddr_un;
 
 #define TOO_LONG (1 + sizeof example_sockaddr_un.sun_path)
 
@@ -69,6 +69,11 @@  TEST(socket_path_overflow_client_connect)
 	d = wl_display_connect(path);
 	assert(d == NULL);
 	assert(errno == ENAMETOOLONG);
+
+	/* This is useless, but prevents a warning about example_sockaddr_un
+	 * being discarded from the compilation unit. */
+	strcpy(example_sockaddr_un.sun_path, "happy now clang?");
+	assert(example_sockaddr_un.sun_path[0] != '\0');
 }
 
 TEST(socket_path_overflow_server_create)

Comments

Hi Dan,

On 29 August 2018 at 07:17, Daniel Stone <daniels@collabora.com> wrote:
> Clang will rightly point out that example_sockaddr_un in socket-test
> will get discarded from the compilation unit as it is completely unused.
> Put in a couple of lines which of no value other than stopping Clang
> from complaining.
>
> Signed-off-by: Daniel Stone <daniels@collabora.com>
> ---
>  tests/socket-test.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/tests/socket-test.c b/tests/socket-test.c
> index e9705628..8d39edce 100644
> --- a/tests/socket-test.c
> +++ b/tests/socket-test.c
> @@ -42,7 +42,7 @@
>   * See `man 7 unix`.
>   */
>
> -static const struct sockaddr_un example_sockaddr_un;
> +static struct sockaddr_un example_sockaddr_un;
>
>  #define TOO_LONG (1 + sizeof example_sockaddr_un.sun_path)
>
> @@ -69,6 +69,11 @@ TEST(socket_path_overflow_client_connect)
>         d = wl_display_connect(path);
>         assert(d == NULL);
>         assert(errno == ENAMETOOLONG);
> +
> +       /* This is useless, but prevents a warning about example_sockaddr_un
> +        * being discarded from the compilation unit. */
> +       strcpy(example_sockaddr_un.sun_path, "happy now clang?");
> +       assert(example_sockaddr_un.sun_path[0] != '\0');

Why don't you add _attrubute__((used)) instead of doing the blame
game? We already use it in the repo.

Side note: the manpage says "sun_path[108]" and also points out that
"some implementations have sun_path as short as 92 bytes"
The check in wl_socket_init_for_display_name uses sizeof, yet prints a
"exceeds 108 bytes" message.

I guess the message should be fixed?

HTH
Emil
Hi Emil,

On Wed, 29 Aug 2018 at 17:33, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> On 29 August 2018 at 07:17, Daniel Stone <daniels@collabora.com> wrote:
> > -static const struct sockaddr_un example_sockaddr_un;
> > +static struct sockaddr_un example_sockaddr_un;
> >
> >  #define TOO_LONG (1 + sizeof example_sockaddr_un.sun_path)
> >
> > @@ -69,6 +69,11 @@ TEST(socket_path_overflow_client_connect)
> >         d = wl_display_connect(path);
> >         assert(d == NULL);
> >         assert(errno == ENAMETOOLONG);
> > +
> > +       /* This is useless, but prevents a warning about example_sockaddr_un
> > +        * being discarded from the compilation unit. */
> > +       strcpy(example_sockaddr_un.sun_path, "happy now clang?");
> > +       assert(example_sockaddr_un.sun_path[0] != '\0');
>
> Why don't you add _attrubute__((used)) instead of doing the blame
> game? We already use it in the repo.

This was already merged, but sure, a follow-up patch to do this would be fine.

> Side note: the manpage says "sun_path[108]" and also points out that
> "some implementations have sun_path as short as 92 bytes"
> The check in wl_socket_init_for_display_name uses sizeof, yet prints a
> "exceeds 108 bytes" message.
>
> I guess the message should be fixed?

Yes, it looks like a follow-up patch to use sizeof would be correct as well.

Cheers,
Daniel