Or we could actually check the version; if tmp is NULL, then send_error(out, 400, "Bad Request"), and if version doesn't start with "HTTP/1.", then send_error(out, 505, "HTTP Version Not Supported");

Also quite a reasonable approach. Should I file a separate bug for each of this type of warning (at least some look like simple removal would work, but no idea if some should remain as placeholders for future code, etc.), or keep all here in this one?

as long as none of them gets too big or controversial I think it's fine to do multiple related patches here

Next one is in gthreadedresolver.c g_resolver_records_from_res_query(), in the "Read answers" loop:

 while (count-- && p < end)
    {
      p += dn_expand (answer, end, p, namebuf, sizeof (namebuf));
      GETSHORT (type, p);
      GETSHORT (qclass, p);
      GETLONG  (ttl, p);
      ttl = ttl; /* To avoid -Wunused-but-set-variable */
      GETSHORT (rdlength, p);


ttl is never used. Data at p is parsed sequentially, so the ttl value needs to be extracted, but then the loop here doesn't use it. GETLONG is NS_GET32, which reads a few bytes from p to create ttl and then increments p to the next chunk of data. I don't know if there is anything useful for ttl here, but if we just want to ignore this value, just increment p with something like (based on NS_GET32 around line 317), which is also comparable to what my /usr/include/nameser.h has):

#define NS_SKIP32(cp) do { \
	(cp) += NS_INT32SZ; \
} while (/*CONSTCOND*/0)

Next one is in gdbusmessage.c parse_value_from_blob(), where is_leaf is set to various boolean values depending on the passed blob type. This variable is apparently only a status used for debugging...the only place its value is used is around line 1879:

#ifdef DEBUG_SERIALIZER
  [do things with is_leaf]
#else
  is_leaf = is_leaf;  // <-- triggers warning
#endif

Probably all uses of is_leaf should be completely protected by that token.

Next one is gdusauth.c _g_dbus_auth_run_server(), where there are two instances (depending on G_OS_UNIX) of:

      byte = g_data_input_stream_read_byte (dis, cancellable, &local_error);
      byte = byte; /* To avoid -Wunused-but-set-variable */

There is a comment near there:

  /* first read the NUL-byte (TODO: read credentials if using a unix domain socket) */

so the code is just reading it into byte and then ignoring it. Should it check that it's actually NUL? Or should it just read it in a void context rather than assigning it to a dummy variable?

A trivial one in gdbusauthmechanismsha1.c keyring_generate_entry() at line 749:

line_when = g_ascii_strtoll (tokens[1], &endp, 10);
[do some things]
line_when = line_when; /* To avoid -Wunused-but-set-variable */
[do some more things]
if (line_when > now)

so it actually *is* used after being set. Previously was probably just awaiting someone to implement the handler for its value. Simply removing the line_when=line_when line solves it.

There are two others in this .c, not as obvious as this one.

Want to wrap this all up in a patch?

(In reply to Daniel Macks from comment #4)
>       GETLONG  (ttl, p);
>       ttl = ttl; /* To avoid -Wunused-but-set-variable */

probably "p += 4" is sufficient

(In reply to Daniel Macks from comment #5)
> Probably all uses of is_leaf should be completely protected by that token.

yes

(In reply to Daniel Macks from comment #6)
> so the code is just reading it into byte and then ignoring it. Should it
> check that it's actually NUL? Or should it just read it in a void context
> rather than assigning it to a dummy variable?

The !G_IS_UNIX_CONNECTION branch doesn't check for NUL, so this branch shouldn't either. (The D-Bus Specification says that "If the first byte received from the client is not a nul byte, the server may disconnect that client". "May", not "must", so just ignoring it is allowed.)

>   /* first read the NUL-byte (TODO: read credentials if using a unix domain socket) */

That TODO is already done...

(In reply to Daniel Macks from comment #7)
> so it actually *is* used after being set. Previously was probably just
> awaiting someone to implement the handler for its value. Simply removing the
> line_when=line_when line solves it.

Yes, sounds right

(In reply to Dan Winship from comment #8)
> (In reply to Daniel Macks from comment #6)
> > so the code is just reading it into byte and then ignoring it. Should it
> > check that it's actually NUL? Or should it just read it in a void context
> > rather than assigning it to a dummy variable?
> 
> The !G_IS_UNIX_CONNECTION branch doesn't check for NUL, so this branch
> shouldn't either. (The D-Bus Specification says that "If the first byte
> received from the client is not a nul byte, the server may disconnect that
> client". "May", not "must", so just ignoring it is allowed.)

So it definitely needs to be ignored. Is just calling (void)g_data_input_stream_read_byte(...) okay here?

sure

Created attachment 299905 [details] [review]
Fix some -Wself-assign warnings

Includes all specific cases and solutions discussed so far. There are still others yet un-analyzed.

Created attachment 302197 [details] [review]
Fix more -Wself-assign warnings

Resolves the two other instances in gdbusauthmechanismsha1.c (followup from Comment #7).

Review of attachment 299905 [details] [review]:

Looks good to me.

Review of attachment 302197 [details] [review]:

Looks good.

Rebased (trivial conflicts to resolve) and pushed.