[NaCl SDK] nacl_io: Remove use of new/delete for data buffers.

[NaCl SDK] nacl_io: Remove use of new/delete for data buffers.

This is the first step in the transition of nacl_io
from C++ to C.

Also, remove an unused method (Packet::Take).

BUG=400779
R=binji@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=288314

[binji, 2014-08-05 19:30:29]

https://codereview.chromium.org/443693002/diff/20001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/fifo_char.cc (right):

https://codereview.chromium.org/443693002/diff/20001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/fifo_char.cc:17: buffer_ =
(char*)malloc(size);
we should probably check for NULL returns for all mallocs/reallocs.

https://codereview.chromium.org/443693002/diff/20001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/memfs/mem_fs_node.cc (right):

https://codereview.chromium.org/443693002/diff/20001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/memfs/mem_fs_node.cc:87: data_len_ =
new_size + extra;
We don't want to always grow. What if new_size < data_len_?

https://codereview.chromium.org/443693002/diff/20001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/memfs/mem_fs_node.cc:91: memset(data_ +
new_size, 0, data_len_ - new_size);
Why? I think it makes more sense to zero from the old size to the new_size (in
case the buffer is reused).

https://codereview.chromium.org/443693002/diff/20001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/memfs/mem_fs_node.h (right):

https://codereview.chromium.org/443693002/diff/20001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/memfs/mem_fs_node.h:37: int data_len_;
size_t

https://codereview.chromium.org/443693002/diff/20001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/memfs/mem_fs_node.h:37: int data_len_;
This should be data_capacity (or something similar). data_len makes me think it
is the length of the actual data, but the resize code below shows that it is
not.

[Sam Clegg, 2014-08-06 09:10:32]

https://codereview.chromium.org/443693002/diff/20001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/fifo_char.cc (right):

https://codereview.chromium.org/443693002/diff/20001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/fifo_char.cc:17: buffer_ =
(char*)malloc(size);
On 2014/08/05 19:30:29, binji wrote:
> we should probably check for NULL returns for all mallocs/reallocs.

Done. Although there is not much useful we can do in 
most cases.

https://codereview.chromium.org/443693002/diff/20001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/memfs/mem_fs_node.cc (right):

https://codereview.chromium.org/443693002/diff/20001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/memfs/mem_fs_node.cc:87: data_len_ =
new_size + extra;
On 2014/08/05 19:30:29, binji wrote:
> We don't want to always grow. What if new_size < data_len_?

Done.

https://codereview.chromium.org/443693002/diff/20001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/memfs/mem_fs_node.cc:91: memset(data_ +
new_size, 0, data_len_ - new_size);
On 2014/08/05 19:30:29, binji wrote:
> Why? I think it makes more sense to zero from the old size to the new_size (in
> case the buffer is reused).

Done.

https://codereview.chromium.org/443693002/diff/20001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/memfs/mem_fs_node.h (right):

https://codereview.chromium.org/443693002/diff/20001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/memfs/mem_fs_node.h:37: int data_len_;
On 2014/08/05 19:30:29, binji wrote:
> size_t

Done.

https://codereview.chromium.org/443693002/diff/20001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/memfs/mem_fs_node.h:37: int data_len_;
On 2014/08/05 19:30:29, binji wrote:
> size_t

Done.

[binji, 2014-08-06 17:18:00]

lgtm

https://codereview.chromium.org/443693002/diff/40001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/fifo_char.cc (right):

https://codereview.chromium.org/443693002/diff/40001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/fifo_char.cc:42: assert(buffer_ !=
NULL);
could return false here?

https://codereview.chromium.org/443693002/diff/40001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/httpfs/http_fs.cc (right):

https://codereview.chromium.org/443693002/diff/40001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/httpfs/http_fs.cc:400: assert(text !=
NULL);
could return error here?

https://codereview.chromium.org/443693002/diff/40001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/memfs/mem_fs_node.cc (right):

https://codereview.chromium.org/443693002/diff/40001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/memfs/mem_fs_node.cc:103: assert(data_
!= NULL);
could return error here?

[Sam Clegg, 2014-08-07 11:49:52]

https://codereview.chromium.org/443693002/diff/40001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/fifo_char.cc (right):

https://codereview.chromium.org/443693002/diff/40001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/fifo_char.cc:42: assert(buffer_ !=
NULL);
On 2014/08/06 17:17:59, binji wrote:
> could return false here?

Done.

https://codereview.chromium.org/443693002/diff/40001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/httpfs/http_fs.cc (right):

https://codereview.chromium.org/443693002/diff/40001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/httpfs/http_fs.cc:400: assert(text !=
NULL);
On 2014/08/06 17:17:59, binji wrote:
> could return error here?

Done.

https://codereview.chromium.org/443693002/diff/40001/native_client_sdk/src/li...
File native_client_sdk/src/libraries/nacl_io/memfs/mem_fs_node.cc (right):

https://codereview.chromium.org/443693002/diff/40001/native_client_sdk/src/li...
native_client_sdk/src/libraries/nacl_io/memfs/mem_fs_node.cc:103: assert(data_
!= NULL);
On 2014/08/06 17:18:00, binji wrote:
> could return error here?

Done.

[binji, 2014-08-07 15:22:32]

https://codereview.chromium.org/443693002/diff/100001/native_client_sdk/src/l...
File native_client_sdk/src/libraries/nacl_io/memfs/mem_fs_node.cc (right):

https://codereview.chromium.org/443693002/diff/100001/native_client_sdk/src/l...
native_client_sdk/src/libraries/nacl_io/memfs/mem_fs_node.cc:76:
LOG_ERROR("memfs: resize %ld failed: %s", new_size, strerror(error));
Looks like you need a special macro for off_t?

memfs/mem_fs_node.cc:76:49: error: format specifies type 'long' but the argument
has type 'off_t' (aka 'long long') [-Werror,-Wformat]
      LOG_ERROR("memfs: resize %ld failed: %s", new_size, strerror(error));
                               ~~~              ^~~~~~~~
                               %lld

[Sam Clegg, 2014-08-07 17:36:54]

On 2014/08/07 15:22:32, binji wrote:
>
https://codereview.chromium.org/443693002/diff/100001/native_client_sdk/src/l...
> File native_client_sdk/src/libraries/nacl_io/memfs/mem_fs_node.cc (right):
> 
>
https://codereview.chromium.org/443693002/diff/100001/native_client_sdk/src/l...
> native_client_sdk/src/libraries/nacl_io/memfs/mem_fs_node.cc:76:
> LOG_ERROR("memfs: resize %ld failed: %s", new_size, strerror(error));
> Looks like you need a special macro for off_t?
> 
> memfs/mem_fs_node.cc:76:49: error: format specifies type 'long' but the
argument
> has type 'off_t' (aka 'long long') [-Werror,-Wformat]
>       LOG_ERROR("memfs: resize %ld failed: %s", new_size, strerror(error));
>                                ~~~              ^~~~~~~~
>                                %lld

Found a few more to remove. PTAL.

[binji, 2014-08-07 17:52:52]

lgtm

https://codereview.chromium.org/443693002/diff/160001/native_client_sdk/src/l...
File native_client_sdk/src/libraries/nacl_io/httpfs/http_fs_node.cc (right):

https://codereview.chromium.org/443693002/diff/160001/native_client_sdk/src/l...
native_client_sdk/src/libraries/nacl_io/httpfs/http_fs_node.cc:566: buffer_len_
= kBytesToRead;
should set buffer_len_ to 0 if !buffer_?

https://codereview.chromium.org/443693002/diff/160001/native_client_sdk/src/l...
native_client_sdk/src/libraries/nacl_io/httpfs/http_fs_node.cc:619: buffer_len_
= new_len;
same here?

[Sam Clegg, 2014-08-08 09:26:37]

https://codereview.chromium.org/443693002/diff/160001/native_client_sdk/src/l...
File native_client_sdk/src/libraries/nacl_io/httpfs/http_fs_node.cc (right):

https://codereview.chromium.org/443693002/diff/160001/native_client_sdk/src/l...
native_client_sdk/src/libraries/nacl_io/httpfs/http_fs_node.cc:566: buffer_len_
= kBytesToRead;
On 2014/08/07 17:52:52, binji wrote:
> should set buffer_len_ to 0 if !buffer_?

Done.

https://codereview.chromium.org/443693002/diff/160001/native_client_sdk/src/l...
native_client_sdk/src/libraries/nacl_io/httpfs/http_fs_node.cc:619: buffer_len_
= new_len;
On 2014/08/07 17:52:52, binji wrote:
> same here?

Done.

[Sam Clegg, 2014-08-08 10:37:22]

Committed patchset #10 manually as 288314 (presubmit successful).