[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: fixes

From: Stefan Küng <tortoisesvn_at_gmail.com>
Date: Fri, 13 Apr 2012 18:18:44 +0200

On 12.04.2012 23:22, abel_at_abel.hu wrote:
> Hi guys,
>
> here are some fixes.
>
> BaseView.patch
> - "line" variable has been set to a temporary CString object's buffer.
> Before the execution of the next line, this temporary object is deleted.

A few issues with this patch:
* uses tabs instead of spaces (easily fixed)
* you're calling the expensive GetLineChars() method unconditionally,
even if there's nothing to do
* in case there's nothing to do, you're adding an extra line (pushing
zero twice to the array)

> - logically dead code removed

> FileTextLines.patch
> - ReadFile does not null-terminate the string it reads. Later pFileBuf
> is used as if it would be null-terminated.

While it's true that ReadFile does not null-terminate the buffer it
reads (doesn't need to be a string), it's not necessary here:
we have the number of read bytes, and create the string we need
accordingly. We don't create the string from start to the terminating
null but from start to the number of read bytes.

Also, if you really want to make sure the buffer is null terminated,
it's not necessary to clear the whole buffer (way too expensive!): you
could just set the byte after the last read byte to zero (after reading).

> LocatorBar.cpp
> - checking m_pMainFrm a few lines upper implies that m_pMainFrm might be
> NULL. In this case dereferencing it means crash.
>
>
> missing_initializers.patch
> - some missing initializers in constructors.

Committed most of your patches with my fixes (and all tabs converted to
spaces).

also: your patches are invalid. Not sure if it's the mailer that messes
them up, but the first lines all have an empty line in between them. SVN
does not recognize those patches.
Not a big deal: I can just remove those extra lines in the patch header
myself.

Stefan

-- 
        ___
   oo  // \\      "De Chelonian Mobile"
  (_,\/ \_/ \     TortoiseSVN
    \ \_/_\_/>    The coolest Interface to (Sub)Version Control
    /_/   \_\     http://tortoisesvn.net
------------------------------------------------------
http://tortoisesvn.tigris.org/ds/viewMessage.do?dsForumId=757&dsMessageId=2948091
To unsubscribe from this discussion, e-mail: [dev-unsubscribe_at_tortoisesvn.tigris.org].
Received on 2012-04-13 18:18:54 CEST

This is an archived mail posted to the TortoiseSVN Dev mailing list.