From: Pedro Alves <palves@redhat.com>
To: Jan Kratochvil <jan.kratochvil@redhat.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH 2/6] Code cleanup: dwarf2read.c: Eliminate ::file_write
Date: Mon, 19 Jun 2017 09:26:00 -0000 [thread overview]
Message-ID: <b7ede527-0d1a-23cd-8eaa-98f7e89a28ef@redhat.com> (raw)
In-Reply-To: <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
next prev parent reply other threads:[~2017-06-19 9:26 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-26 18:25 [PATCH 0/6] DWARF-5: .debug_names index Jan Kratochvil
2017-05-26 18:25 ` [PATCH 2/6] cc-with-tweaks.sh: Use gdb-add-index.sh Jan Kratochvil
2017-05-26 18:25 ` [PATCH 1/6] Code cleanup: C++ify .gdb_index producer Jan Kratochvil
2017-06-12 16:08 ` [pushed] " Pedro Alves
2017-06-12 16:14 ` [PATCH 1/6] Code cleanup: dwarf2read.c:uniquify_cu_indices: Use std::unique Pedro Alves
2017-06-12 16:14 ` [PATCH 3/6] Code cleanup: dwarf2read.c: Add data_buf::append_uint Pedro Alves
2017-06-12 16:14 ` [PATCH 4/6] .gdb_index prod perf regression: find before insert in unordered_map Pedro Alves
2017-06-12 16:14 ` [PATCH 2/6] Code cleanup: dwarf2read.c: Eliminate ::file_write Pedro Alves
2017-06-17 17:35 ` Jan Kratochvil
2017-06-19 9:26 ` Pedro Alves [this message]
2017-06-18 18:36 ` Regression: " Jan Kratochvil
2017-06-19 9:27 ` Pedro Alves
2017-06-19 9:39 ` Jan Kratochvil
2017-06-19 9:47 ` Pedro Alves
2017-06-19 10:03 ` Jan Kratochvil
2017-06-19 10:35 ` Pedro Alves
2017-06-19 12:06 ` Yao Qi
2017-06-19 12:26 ` Jan Kratochvil
2017-06-12 16:14 ` [PATCH 6/6] .gdb_index prod perf regression: mapped_symtab now vector of values Pedro Alves
2017-06-12 16:18 ` [pushed] Re: [PATCH 1/6] Code cleanup: C++ify .gdb_index producer Pedro Alves
2017-06-12 16:19 ` [PATCH 5/6] .gdb_index prod perf regression: Estimate size of psyms_seen Pedro Alves
2017-06-18 14:25 ` [pushed] Re: [PATCH 1/6] Code cleanup: C++ify .gdb_index producer Jan Kratochvil
2017-06-18 15:12 ` Eli Zaretskii
2017-06-19 11:50 ` [pushed] .gdb_index writer: close the file before unlinking it (Re: [pushed] Re: [PATCH 1/6] Code cleanup: C++ify .gdb_index producer.) Pedro Alves
2017-06-18 16:50 ` [pushed] Re: [PATCH 1/6] Code cleanup: C++ify .gdb_index producer Jan Kratochvil
2017-05-26 18:26 ` [PATCH 4/6] Code cleanup: dwarf2_initialize_objfile return value Jan Kratochvil
2017-05-26 18:26 ` [PATCH 3/6] DWARF-5: .debug_names index producer Jan Kratochvil
2017-06-09 5:58 ` [PATCH 3.1/6] " Jan Kratochvil
2017-05-26 18:26 ` [PATCH 6/6] DWARF-5: .debug_names index consumer Jan Kratochvil
2017-05-26 18:26 ` [PATCH 5/6] Refactor: Move some generic code out of .gdb_index code Jan Kratochvil
2017-06-18 19:37 ` obsolete: [PATCH 0/6] DWARF-5: .debug_names index Jan Kratochvil
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b7ede527-0d1a-23cd-8eaa-98f7e89a28ef@redhat.com \
--to=palves@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=jan.kratochvil@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox