Add utime implementation to libnacl

Add utime implementation to libnacl

This relies on the IRT utimes function to implement utime.
Although the sel_ldr syscall is not yet implemented this
change allows nacl_io to intercept utime calls without
overriding utime itself.

R=mseaborn@chromium.org
BUG= https://code.google.com/p/chromium/issues/detail?id=387487

Committed: https://chromium.googlesource.com/native_client/src/native_client/+/405975899a17e58d3541cbd59ba1bb89600d1f4a

[Mark Seaborn, 2015-04-07 19:08:43]

LGTM with some tweaks...

https://codereview.chromium.org/1065963002/diff/20001/src/untrusted/nacl/sys_...
File src/untrusted/nacl/sys_private.c (right):

https://codereview.chromium.org/1065963002/diff/20001/src/untrusted/nacl/sys_...
src/untrusted/nacl/sys_private.c:256: int utime(const char *path, const struct
utimbuf *buf) {
If utime.c calls utimes(), you don't need this here.

https://codereview.chromium.org/1065963002/diff/20001/src/untrusted/nacl/sys_...
src/untrusted/nacl/sys_private.c:258: struct timeval* tv = NULL;
Use " *" spacing

https://codereview.chromium.org/1065963002/diff/20001/src/untrusted/nacl/utime.c
File src/untrusted/nacl/utime.c (right):

https://codereview.chromium.org/1065963002/diff/20001/src/untrusted/nacl/utim...
src/untrusted/nacl/utime.c:11: #include <errno.h>
Sort #includes

https://codereview.chromium.org/1065963002/diff/20001/src/untrusted/nacl/utim...
src/untrusted/nacl/utime.c:12: #include <stdio.h>
Not needed

https://codereview.chromium.org/1065963002/diff/20001/src/untrusted/nacl/utim...
src/untrusted/nacl/utime.c:14: #include
"native_client/src/untrusted/nacl/nacl_irt.h"
Not needed with the change below...

https://codereview.chromium.org/1065963002/diff/20001/src/untrusted/nacl/utim...
src/untrusted/nacl/utime.c:19: * second (support time_t).  This means we we can
use utimes to impelement
What do you mean by "support time_t"?  Can you remove that parenthetical comment
or clean it up?

Also: "implement"

https://codereview.chromium.org/1065963002/diff/20001/src/untrusted/nacl/utim...
src/untrusted/nacl/utime.c:29: struct timeval* tv = NULL;
Use " *" spacing

https://codereview.chromium.org/1065963002/diff/20001/src/untrusted/nacl/utim...
src/untrusted/nacl/utime.c:38: int error =
__libnacl_irt_dev_filename.utimes(filename, tv);
Just call utimes() instead of getting the IRT interface.  Then you don't need
the sys_private.c change.

https://codereview.chromium.org/1065963002/diff/20001/tests/syscalls/syscalls.cc
File tests/syscalls/syscalls.cc (right):

https://codereview.chromium.org/1065963002/diff/20001/tests/syscalls/syscalls...
tests/syscalls/syscalls.cc:1100: ret &= test_utime(test_file);
Nit: Put after test_utimes() to match the definitions

[Sam Clegg, 2015-04-10 21:18:56]

https://codereview.chromium.org/1065963002/diff/20001/src/untrusted/nacl/sys_...
File src/untrusted/nacl/sys_private.c (right):

https://codereview.chromium.org/1065963002/diff/20001/src/untrusted/nacl/sys_...
src/untrusted/nacl/sys_private.c:256: int utime(const char *path, const struct
utimbuf *buf) {
On 2015/04/07 19:08:43, Mark Seaborn wrote:
> If utime.c calls utimes(), you don't need this here.

Good idea.  Done.

https://codereview.chromium.org/1065963002/diff/20001/src/untrusted/nacl/sys_...
src/untrusted/nacl/sys_private.c:258: struct timeval* tv = NULL;
On 2015/04/07 19:08:43, Mark Seaborn wrote:
> Use " *" spacing

Acknowledged.

https://codereview.chromium.org/1065963002/diff/20001/src/untrusted/nacl/utime.c
File src/untrusted/nacl/utime.c (right):

https://codereview.chromium.org/1065963002/diff/20001/src/untrusted/nacl/utim...
src/untrusted/nacl/utime.c:11: #include <errno.h>
On 2015/04/07 19:08:43, Mark Seaborn wrote:
> Sort #includes

Done.

https://codereview.chromium.org/1065963002/diff/20001/src/untrusted/nacl/utim...
src/untrusted/nacl/utime.c:12: #include <stdio.h>
On 2015/04/07 19:08:43, Mark Seaborn wrote:
> Not needed

Done.

https://codereview.chromium.org/1065963002/diff/20001/src/untrusted/nacl/utim...
src/untrusted/nacl/utime.c:14: #include
"native_client/src/untrusted/nacl/nacl_irt.h"
On 2015/04/07 19:08:43, Mark Seaborn wrote:
> Not needed with the change below...

Done.

https://codereview.chromium.org/1065963002/diff/20001/src/untrusted/nacl/utim...
src/untrusted/nacl/utime.c:19: * second (support time_t).  This means we we can
use utimes to impelement
On 2015/04/07 19:08:43, Mark Seaborn wrote:
> What do you mean by "support time_t"?  Can you remove that parenthetical
comment
> or clean it up?
> 
> Also: "implement"

Done.

https://codereview.chromium.org/1065963002/diff/20001/src/untrusted/nacl/utim...
src/untrusted/nacl/utime.c:29: struct timeval* tv = NULL;
On 2015/04/07 19:08:43, Mark Seaborn wrote:
> Use " *" spacing

Done.

https://codereview.chromium.org/1065963002/diff/20001/src/untrusted/nacl/utim...
src/untrusted/nacl/utime.c:38: int error =
__libnacl_irt_dev_filename.utimes(filename, tv);
On 2015/04/07 19:08:43, Mark Seaborn wrote:
> Just call utimes() instead of getting the IRT interface.  Then you don't need
> the sys_private.c change.

Done.

https://codereview.chromium.org/1065963002/diff/20001/tests/syscalls/syscalls.cc
File tests/syscalls/syscalls.cc (right):

https://codereview.chromium.org/1065963002/diff/20001/tests/syscalls/syscalls...
tests/syscalls/syscalls.cc:1100: ret &= test_utime(test_file);
On 2015/04/07 19:08:43, Mark Seaborn wrote:
> Nit: Put after test_utimes() to match the definitions

Done.

[Mark Seaborn, 2015-04-15 18:14:18]

LGTM with comment fixes.

https://codereview.chromium.org/1065963002/diff/40001/src/untrusted/nacl/utime.c
File src/untrusted/nacl/utime.c (right):

https://codereview.chromium.org/1065963002/diff/40001/src/untrusted/nacl/utim...
src/untrusted/nacl/utime.c:13: * TODO(sbc): remove this once utimes declaration
get added to the newlib
"gets"

https://codereview.chromium.org/1065963002/diff/40001/src/untrusted/nacl/utim...
src/untrusted/nacl/utime.c:19: * Implementation of utime based on the utimes. 
utime works just like utimes
"the utimes" -> "utimes"

Suggestion: adding "()" to function names makes it clear you're referring to
functions (i.e. "utimes()")

https://codereview.chromium.org/1065963002/diff/40001/src/untrusted/nacl/utim...
src/untrusted/nacl/utime.c:20: * but only support timestamps with a granularity
of one second (time_t). This
"supports"

https://codereview.chromium.org/1065963002/diff/40001/src/untrusted/nacl/utim...
src/untrusted/nacl/utime.c:21: * means we we can use utimes to impelement utime
by simply setting tv_sec
"implement".  Also, you mean "tv_usec" not "tv_sec".

[Sam Clegg, 2015-04-15 20:46:40]

https://codereview.chromium.org/1065963002/diff/40001/src/untrusted/nacl/utime.c
File src/untrusted/nacl/utime.c (right):

https://codereview.chromium.org/1065963002/diff/40001/src/untrusted/nacl/utim...
src/untrusted/nacl/utime.c:13: * TODO(sbc): remove this once utimes declaration
get added to the newlib
On 2015/04/15 18:14:18, Mark Seaborn wrote:
> "gets"

Done.

https://codereview.chromium.org/1065963002/diff/40001/src/untrusted/nacl/utim...
src/untrusted/nacl/utime.c:19: * Implementation of utime based on the utimes. 
utime works just like utimes
On 2015/04/15 18:14:18, Mark Seaborn wrote:
> "the utimes" -> "utimes"
> 
> Suggestion: adding "()" to function names makes it clear you're referring to
> functions (i.e. "utimes()")

Done.

https://codereview.chromium.org/1065963002/diff/40001/src/untrusted/nacl/utim...
src/untrusted/nacl/utime.c:20: * but only support timestamps with a granularity
of one second (time_t). This
On 2015/04/15 18:14:18, Mark Seaborn wrote:
> "supports"

Done.

https://codereview.chromium.org/1065963002/diff/40001/src/untrusted/nacl/utim...
src/untrusted/nacl/utime.c:21: * means we we can use utimes to impelement utime
by simply setting tv_sec
On 2015/04/15 18:14:18, Mark Seaborn wrote:
> "implement".  Also, you mean "tv_usec" not "tv_sec".

Done.

[Sam Clegg, 2015-04-15 20:46:45]

The CQ bit was checked by sbc@chromium.org

[Sam Clegg, 2015-04-15 20:46:45]

The patchset sent to the CQ was uploaded after l-g-t-m from
mseaborn@chromium.org
Link to the patchset: https://codereview.chromium.org/1065963002/#ps60001
(title: " ")

[commit-bot: I haz the power, 2015-04-15 20:46:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1065963002/60001

[commit-bot: I haz the power, 2015-04-15 22:57:50]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/native_client/src/native_client/+/405975899...