This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 2/6] Code cleanup: dwarf2read.c: Eliminate ::file_write
- From: Pedro Alves <palves at redhat dot com>
- To: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Mon, 19 Jun 2017 10:26:29 +0100
- Subject: Re: [PATCH 2/6] Code cleanup: dwarf2read.c: Eliminate ::file_write
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx02.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx02.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=palves at redhat dot com
- Dkim-filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 9CABC80C11
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 9CABC80C11
- References: <8efc0742-1014-4fe0-6948-f40a9c5c4975@redhat.com> <1497284051-13795-2-git-send-email-palves@redhat.com> <20170617173443.GA17084@host1.jankratochvil.net>
On 06/17/2017 06:34 PM, Jan Kratochvil wrote:
> On Mon, 12 Jun 2017 18:14:07 +0200, Pedro Alves wrote:
>> There's no real need for all this indirection.
>
>> - ::file_write (file, m_vec);
>> + if (::fwrite (m_vec.data (), 1, m_vec.size (), file) != m_vec.size ())
>> + error (_("couldn't write data to file"));
>
> Aren't 28 characters with the variable mentioned once easier to write and more
> bug-proof than to write each time 115 characters
That's not a correct comparison, because you're not accounting for all the
characters of the wrapper functions there were removed. If you count those,
then certainly we end up with fewer characters overall. It might be different if
the wrapper functions were used in more than one place, but they aren't.
> mentioning the same variable 3 times?
The old code mentioned the same variable 3 times as well:
-template<class Elem>
-static void
-file_write (FILE *file, const std::vector<Elem> &vec)
-{
- file_write (file, vec.data (), vec.size() * sizeof (vec[0]));
^^^ ^^^ ^^^
-}
I ran into this because with the byte_vector change, we'd have to
to modify this template's template parameter and prototype.
So the extra indirection ended up getting in the way, and it's
much simpler to just get rid of it.
Thanks,
Pedro Alves