This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 2/3] Fix copy_bitwise()
On 11/14/2016 11:54 AM, Andreas Arnez wrote:
On Mon, Nov 14 2016, Luis Machado wrote:
On 11/14/2016 09:02 AM, Andreas Arnez wrote:
[...]
+ dest += dest_offset / 8;
+ dest_offset %= 8;
+ source += source_offset / 8;
+ source_offset %= 8;
Are you sure you will always have non-zero source_offset and dest_offset
when explicitly dividing them by 8? If i were to feed (or GDB, in some
erroneous state) invalid data to the function, this would likely crash?
There are other cases of explicit / operations.
No, copy_bitwise should work fine with source_offset and dest_offset set
to zero. Where do you think it would crash?
Well, obviously i wasn't fully awake. Nevermind this.
[...]
+ /* Fill BUF with DEST_OFFSET bits from the destination and 8 -
+ SOURCE_OFFSET bits from the source. */
+ buf = *(bits_big_endian ? source-- : source++) >> source_offset;
Maybe it's just me, but having constructs like the above don't help much
performance-wise and make the code slightly less readable. Should we
expand this further? There are multiple occurrences of this.
Well, I've tried a few different ways and found this approach actually
the easiest to read, for my taste. For instance, it makes the multiple
occurrences easy to recognize -- as you pointed out ;-)
Of course, if people feel that this post-decrement/increment pattern
really hurts readability, I can provide a more "stretched" form instead.
No strong opinions there. Just a suggestion.