Upgrading lzma_sdk to version 9.20.

Upgrading lzma_sdk to version 9.20.


BUG=None
TEST=None

[robertshield, 2011-03-25 01:35:01]

LGTM

http://codereview.chromium.org/6730044/diff/1/chrome/installer/util/lzma_util.cc
File chrome/installer/util/lzma_util.cc (right):

http://codereview.chromium.org/6730044/diff/1/chrome/installer/util/lzma_util...
chrome/installer/util/lzma_util.cc:14: #include "third_party/lzma_sdk/C/7zCrc.h"
If the order of includes here isn't important then they should go in alphabetic
order

[bradn, 2011-03-25 02:10:27]

Ok, so I misread my compile result and realized the interface has actually
changed a good bit here.

Please take another look.
In particular I'm a little unsure about the filename conversions.

http://codereview.chromium.org/6730044/diff/1/chrome/installer/util/lzma_util.cc
File chrome/installer/util/lzma_util.cc (right):

http://codereview.chromium.org/6730044/diff/1/chrome/installer/util/lzma_util...
chrome/installer/util/lzma_util.cc:14: #include "third_party/lzma_sdk/C/7zCrc.h"
On 2011/03/25 01:35:01, robertshield wrote:
> If the order of includes here isn't important then they should go in
alphabetic
> order

Done.

[robertshield, 2011-03-25 02:34:37]

looking pretty good, some nits:

http://codereview.chromium.org/6730044/diff/4012/chrome/installer/util/lzma_u...
File chrome/installer/util/lzma_util.cc (right):

http://codereview.chromium.org/6730044/diff/4012/chrome/installer/util/lzma_u...
chrome/installer/util/lzma_util.cc:71: CFileInStream archiveStream;
archiveStream -> archive_stream

http://codereview.chromium.org/6730044/diff/4012/chrome/installer/util/lzma_u...
chrome/installer/util/lzma_util.cc:72: CLookToRead lookStream;
lookStream -> look_stream

http://codereview.chromium.org/6730044/diff/4012/chrome/installer/util/lzma_u...
chrome/installer/util/lzma_util.cc:73: ISzAlloc allocImp;
alloc_imp

http://codereview.chromium.org/6730044/diff/4012/chrome/installer/util/lzma_u...
chrome/installer/util/lzma_util.cc:74: ISzAlloc allocTempImp;
alloc_temp_imp

http://codereview.chromium.org/6730044/diff/4012/chrome/installer/util/lzma_u...
chrome/installer/util/lzma_util.cc:77: UInt16 *name = NULL;
nit: UInt16* name

http://codereview.chromium.org/6730044/diff/4012/chrome/installer/util/lzma_u...
chrome/installer/util/lzma_util.cc:78: size_t nameLen = 0;
name_len

http://codereview.chromium.org/6730044/diff/4012/chrome/installer/util/lzma_u...
chrome/installer/util/lzma_util.cc:106: CSzFileItem *f = db.db.Files + i;
CSzFileItem* f =...

http://codereview.chromium.org/6730044/diff/4012/chrome/installer/util/lzma_u...
chrome/installer/util/lzma_util.cc:117: if (len > nameLen) {
nameLen here can only be 0, and this is also its first use. Perhaps move the
initial definition of name_len down to line 116?

http://codereview.chromium.org/6730044/diff/4012/chrome/installer/util/lzma_u...
chrome/installer/util/lzma_util.cc:120: name = (UInt16 *)SzAlloc(NULL, nameLen *
sizeof(UInt16));
According to style guide, should be reinterpret_cast<UInt16*>(...)

http://codereview.chromium.org/6730044/diff/4012/chrome/installer/util/lzma_u...
chrome/installer/util/lzma_util.cc:165: (const FILETIME *)&(f->MTime))) {
no C-style casts

[bradn, 2011-03-25 02:53:42]

PTAL

http://codereview.chromium.org/6730044/diff/4012/chrome/installer/util/lzma_u...
File chrome/installer/util/lzma_util.cc (right):

http://codereview.chromium.org/6730044/diff/4012/chrome/installer/util/lzma_u...
chrome/installer/util/lzma_util.cc:71: CFileInStream archiveStream;
On 2011/03/25 02:34:37, robertshield wrote:
> archiveStream -> archive_stream

Done.

http://codereview.chromium.org/6730044/diff/4012/chrome/installer/util/lzma_u...
chrome/installer/util/lzma_util.cc:72: CLookToRead lookStream;
On 2011/03/25 02:34:37, robertshield wrote:
> lookStream -> look_stream

Done.

http://codereview.chromium.org/6730044/diff/4012/chrome/installer/util/lzma_u...
chrome/installer/util/lzma_util.cc:72: CLookToRead lookStream;
On 2011/03/25 02:34:37, robertshield wrote:
> lookStream -> look_stream

Done.

http://codereview.chromium.org/6730044/diff/4012/chrome/installer/util/lzma_u...
chrome/installer/util/lzma_util.cc:73: ISzAlloc allocImp;
On 2011/03/25 02:34:37, robertshield wrote:
> alloc_imp

Done.

http://codereview.chromium.org/6730044/diff/4012/chrome/installer/util/lzma_u...
chrome/installer/util/lzma_util.cc:74: ISzAlloc allocTempImp;
On 2011/03/25 02:34:37, robertshield wrote:
> alloc_temp_imp

Done.

http://codereview.chromium.org/6730044/diff/4012/chrome/installer/util/lzma_u...
chrome/installer/util/lzma_util.cc:77: UInt16 *name = NULL;
On 2011/03/25 02:34:37, robertshield wrote:
> nit: UInt16* name

Done.

http://codereview.chromium.org/6730044/diff/4012/chrome/installer/util/lzma_u...
chrome/installer/util/lzma_util.cc:78: size_t nameLen = 0;
On 2011/03/25 02:34:37, robertshield wrote:
> name_len

Done.

http://codereview.chromium.org/6730044/diff/4012/chrome/installer/util/lzma_u...
chrome/installer/util/lzma_util.cc:106: CSzFileItem *f = db.db.Files + i;
On 2011/03/25 02:34:37, robertshield wrote:
> CSzFileItem* f =...

Done.

http://codereview.chromium.org/6730044/diff/4012/chrome/installer/util/lzma_u...
chrome/installer/util/lzma_util.cc:117: if (len > nameLen) {
On 2011/03/25 02:34:37, robertshield wrote:
> nameLen here can only be 0, and this is also its first use. Perhaps move the
> initial definition of name_len down to line 116?

Rearranged.

http://codereview.chromium.org/6730044/diff/4012/chrome/installer/util/lzma_u...
chrome/installer/util/lzma_util.cc:120: name = (UInt16 *)SzAlloc(NULL, nameLen *
sizeof(UInt16));
On 2011/03/25 02:34:37, robertshield wrote:
> According to style guide, should be reinterpret_cast<UInt16*>(...)

Done.

http://codereview.chromium.org/6730044/diff/4012/chrome/installer/util/lzma_u...
chrome/installer/util/lzma_util.cc:165: (const FILETIME *)&(f->MTime))) {
On 2011/03/25 02:34:37, robertshield wrote:
> no C-style casts

Done.

[robertshield, 2011-03-25 03:23:00]

LGTM

http://codereview.chromium.org/6730044/diff/8001/chrome/installer/util/lzma_u...
File chrome/installer/util/lzma_util.cc (right):

http://codereview.chromium.org/6730044/diff/8001/chrome/installer/util/lzma_u...
chrome/installer/util/lzma_util.cc:161: reinterpret_cast<FILETIME
*>(&(f->MTime)))) {
micro-nit: FILETIME*

[bradn, 2011-03-30 19:12:38]

Got unit tests working.
PTAL.

[robertshield, 2011-03-30 19:40:35]

On 2011/03/30 19:12:38, bradn wrote:
> Got unit tests working.
> PTAL.

New changes LGTM. Note that there is an odd-looking compile failure in the new
code on the linux trybot.

[bradn, 2011-03-30 21:14:57]

Still looking into the other platforms.
Added new compression flags from something laforge and I figured out.
(7zip give better compression with the exe pre-filter on, but this only happens
by default if the input ends in .exe / .dll).

PTAL

In particular at
http://codereview.chromium.org/6730044/diff/23001/chrome/tools/build/win/crea...

[robertshield, 2011-03-30 21:29:36]

On 2011/03/30 21:14:57, bradn wrote:
> Still looking into the other platforms.
> Added new compression flags from something laforge and I figured out.
> (7zip give better compression with the exe pre-filter on, but this only
happens
> by default if the input ends in .exe / .dll).
> 
> PTAL
> 
> In particular at
>
http://codereview.chromium.org/6730044/diff/23001/chrome/tools/build/win/crea...

Neat, still LGTM