Re: [Qemu-devel] [PATCH] readline: avoid memcpy() of overlapping regions

[Stefan Hajnoczi]

On Thu, Jan 17, 2013 at 08:13:38PM +0000, Blue Swirl wrote:
> On Mon, Jan 14, 2013 at 9:09 AM, Stefan Hajnoczi <address@hidden> wrote:
> > On Sat, Jan 12, 2013 at 10:46:18AM +0000, Blue Swirl wrote:
> >> On Thu, Jan 10, 2013 at 12:43 PM, Stefan Hajnoczi <address@hidden> wrote:
> >> > On Wed, Jan 09, 2013 at 08:43:51PM +0000, Blue Swirl wrote:
> >> >> On Tue, Jan 8, 2013 at 9:03 AM, Stefan Hajnoczi <address@hidden> wrote:
> >> >> > On Mon, Jan 07, 2013 at 03:38:39PM -0500, Nickolai Zeldovich wrote:
> >> >> >> memcpy() for overlapping regions is undefined behavior; use memmove()
> >> >> >> instead in readline_hist_add().
> >> >> >>
> >> >> >> Signed-off-by: Nickolai Zeldovich <address@hidden>
> >> >> >> ---
> >> >> >>  readline.c |    4 ++--
> >> >> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >> >> >
> >> >> > I made a slight modification: keep the tab characters since the
> >> >> > surrounding code still uses them.
> >> >>
> >> >> I think tabs should be fixed whenever possible, otherwise we may never
> >> >> get them converted.
> >> >
> >> > Not in a one-line patch when the surrounding lines still use them.  It
> >> > creates a mess.
> >>
> >> Only if the reader messes with the tab width settings (and in that
> >> case they deserve what they get and they are probably also used to
> >> this), otherwise a line with tabs converted to spaces looks exactly
> >> the same.
> >
> > You are oversimplifying how tab widths work.  The author and reader's
> > tab width must match in order for displayed text to appear correctly.
> 
> Exactly. The default tab width is 8 in all tools and it takes some
> effort to adjust the tab settings in each tool. For example, how do
> you change it in less, xterm or cmd.exe?
> 
> It is unreasonable and arrogant for an author to assume any other
> setting used by the reader for tab width, even if this was declared
> for example at the start of the file.
> 
> Perhaps an analogy could be a compressing or encrypting preprocessor
> for the white space in the code. Without the right tool and correct
> settings, the reader would not see the white space in the code
> correctly.
> 
> > Tell me what you consider the "correct" tab width for readers and I'll
> > find a piece of QEMU code that was authored for a different tab width
> > :).
> 
> 8.
> 
> >
> > In other words, it's a mess and best not to perturb it further.
> 
> No, those messes should be cleaned up, much like braces.

Agreed but in cleanup patches or when touching the whole function.  Not
one line at a time because then we have an even bigger mess.

Stefan