third_party: Add the libsync library from Android

third_party: Add the libsync library from Android

BUG=b/27447486

Committed: https://crrev.com/8d6342181e81b6dd0988283b785d0724c686f096
Cr-Commit-Position: refs/heads/master@{#381989}

[reveman, 2016-03-02 16:01:14]

reveman@chromium.org changed reviewers:
+ olofj@chromium.org

[Olof Johansson, 2016-03-02 17:17:22]

https://codereview.chromium.org/1754293002/diff/1/third_party/libsync/include...
File third_party/libsync/include/linux/sw_sync.h (right):

https://codereview.chromium.org/1754293002/diff/1/third_party/libsync/include...
third_party/libsync/include/linux/sw_sync.h:6: * may be copied, distributed, and
modified under those terms.
Can you import GPLv2 sources in Chromium? I don't think you can, so you probably
need to a clean implementation of this file, right?

[reveman, 2016-03-02 18:00:07]

https://codereview.chromium.org/1754293002/diff/1/third_party/libsync/include...
File third_party/libsync/include/linux/sw_sync.h (right):

https://codereview.chromium.org/1754293002/diff/1/third_party/libsync/include...
third_party/libsync/include/linux/sw_sync.h:6: * may be copied, distributed, and
modified under those terms.
On 2016/03/02 at 17:17:21, Olof Johansson wrote:
> Can you import GPLv2 sources in Chromium? I don't think you can, so you
probably need to a clean implementation of this file, right?

Right. Latest patch will just have us release these headers under the Apache 2
license.

[reveman, 2016-03-03 01:02:57]

reveman@chromium.org changed reviewers:
+ xam@google.com

[reveman, 2016-03-03 01:02:57]

+xam

[reveman, 2016-03-03 19:17:32]

reveman@chromium.org changed reviewers:
+ piman@chromium.org

[reveman, 2016-03-03 19:17:33]

+piman

[reveman, 2016-03-03 19:17:40]

The CQ bit was checked by reveman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-03 19:18:21]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1754293002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1754293002/20001

[piman, 2016-03-03 19:43:36]

https://codereview.chromium.org/1754293002/diff/20001/third_party/libsync/OWNERS
File third_party/libsync/OWNERS (right):

https://codereview.chromium.org/1754293002/diff/20001/third_party/libsync/OWN...
third_party/libsync/OWNERS:1: reveman@chromium.org
I think we generally want to put 2 owners in third_party subdirs. You can add
me, or anyone else.

https://codereview.chromium.org/1754293002/diff/20001/third_party/libsync/lib...
File third_party/libsync/libsync.gyp (right):

https://codereview.chromium.org/1754293002/diff/20001/third_party/libsync/lib...
third_party/libsync/libsync.gyp:21: 'defines': ['strlcpy=strncpy'],
ugh - can you add a TODO/file a bug?
It shouldn't be difficult to implement strlcpy(dst, src, n) on top of
strncpy(dst, src, n-1) + explicit termination.

[commit-bot: I haz the power, 2016-03-03 20:04:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-03 20:04:03]

Dry run: This issue passed the CQ dry run.

[reveman, 2016-03-03 20:43:10]

ptal

https://codereview.chromium.org/1754293002/diff/20001/third_party/libsync/OWNERS
File third_party/libsync/OWNERS (right):

https://codereview.chromium.org/1754293002/diff/20001/third_party/libsync/OWN...
third_party/libsync/OWNERS:1: reveman@chromium.org
On 2016/03/03 at 19:43:36, piman - OOO back 2016-03-04 wrote:
> I think we generally want to put 2 owners in third_party subdirs. You can add
me, or anyone else.

Done.

https://codereview.chromium.org/1754293002/diff/20001/third_party/libsync/lib...
File third_party/libsync/libsync.gyp (right):

https://codereview.chromium.org/1754293002/diff/20001/third_party/libsync/lib...
third_party/libsync/libsync.gyp:21: 'defines': ['strlcpy=strncpy'],
On 2016/03/03 at 19:43:36, piman - OOO back 2016-03-04 wrote:
> ugh - can you add a TODO/file a bug?

Done.

> It shouldn't be difficult to implement strlcpy(dst, src, n) on top of
strncpy(dst, src, n-1) + explicit termination.

Yes, I'm just not sure how to best do this without causing problems on platforms
what have a libc that provide this function already.

[piman, 2016-03-04 20:38:19]

lgtm

https://codereview.chromium.org/1754293002/diff/20001/third_party/libsync/lib...
File third_party/libsync/libsync.gyp (right):

https://codereview.chromium.org/1754293002/diff/20001/third_party/libsync/lib...
third_party/libsync/libsync.gyp:21: 'defines': ['strlcpy=strncpy'],
On 2016/03/03 20:43:10, reveman wrote:
> On 2016/03/03 at 19:43:36, piman - OOO back 2016-03-04 wrote:
> > ugh - can you add a TODO/file a bug?
> 
> Done.
> 
> > It shouldn't be difficult to implement strlcpy(dst, src, n) on top of
> strncpy(dst, src, n-1) + explicit termination.
> 
> Yes, I'm just not sure how to best do this without causing problems on
platforms
> what have a libc that provide this function already.

Maybe -Dstrlcpy=strlcpy_chromium and then implement strlcpy_chromium to be
either strlcpy or the strncpy workaround?
Or simply provide strlcpy somewhere in chromium, but #ifdef-ed out on platforms
that already have it in the libc?

[reveman, 2016-03-04 20:43:35]

On 2016/03/04 at 20:38:19, piman wrote:
> lgtm
> 
>
https://codereview.chromium.org/1754293002/diff/20001/third_party/libsync/lib...
> File third_party/libsync/libsync.gyp (right):
> 
>
https://codereview.chromium.org/1754293002/diff/20001/third_party/libsync/lib...
> third_party/libsync/libsync.gyp:21: 'defines': ['strlcpy=strncpy'],
> On 2016/03/03 20:43:10, reveman wrote:
> > On 2016/03/03 at 19:43:36, piman - OOO back 2016-03-04 wrote:
> > > ugh - can you add a TODO/file a bug?
> > 
> > Done.
> > 
> > > It shouldn't be difficult to implement strlcpy(dst, src, n) on top of
> > strncpy(dst, src, n-1) + explicit termination.
> > 
> > Yes, I'm just not sure how to best do this without causing problems on
platforms
> > what have a libc that provide this function already.
> 
> Maybe -Dstrlcpy=strlcpy_chromium and then implement strlcpy_chromium to be
either strlcpy or the strncpy workaround?
> Or simply provide strlcpy somewhere in chromium, but #ifdef-ed out on
platforms that already have it in the libc?

We have base::strlcpy already so maybe just add a #ifdef-ed ::strlcpy wrapper
around that if libc doesn't have it. I'll look at this as a follow up.

[reveman, 2016-03-04 20:46:43]

reveman@chromium.org changed reviewers:
+ thakis@chromium.org

[reveman, 2016-03-04 20:46:44]

+thakis for third_party/

[Nico, 2016-03-04 20:52:05]

you have security and legal approval, yes? (given the size of this, both should
be easy and quick)

https://codereview.chromium.org/1754293002/diff/40001/third_party/libsync/BUI...
File third_party/libsync/BUILD.gn (right):

https://codereview.chromium.org/1754293002/diff/40001/third_party/libsync/BUI...
third_party/libsync/BUILD.gn:19: defines += [ "strlcpy=strncpy" ]
ugh :-(

strlcpy and strncpy have pretty different semantics. can you add a local file
that just implements strlcpy for this project? how hard would it be to change
upstream?

which platform is this needed for?

https://codereview.chromium.org/1754293002/diff/40001/third_party/libsync/REA...
File third_party/libsync/README.chromium (right):

https://codereview.chromium.org/1754293002/diff/40001/third_party/libsync/REA...
third_party/libsync/README.chromium:6: License: Apache 2.0
describe your local changes and how to update this library here

[reveman, 2016-03-07 17:18:25]

On 2016/03/04 at 20:52:05, thakis wrote:
> you have security and legal approval, yes? (given the size of this, both
should be easy and quick)

Linux header re-licensing has been approved but the addition of strlcpy from
OpenBSD has not so I sent out emails for both security and legal approval.

https://codereview.chromium.org/1754293002/diff/40001/third_party/libsync/BUI...
File third_party/libsync/BUILD.gn (right):

https://codereview.chromium.org/1754293002/diff/40001/third_party/libsync/BUI...
third_party/libsync/BUILD.gn:19: defines += [ "strlcpy=strncpy" ]
On 2016/03/04 at 20:52:04, Nico wrote:
> ugh :-(
> 
> strlcpy and strncpy have pretty different semantics. can you add a local file
that just implements strlcpy for this project?

Done. Added the OpenBSD version of strlcpy.

> how hard would it be to change upstream?

Add strlcpy to glibc or change Android? glibc change was proposed 15 years ago
and is still a hot topic so I wouldn't count on that [1]. Android has wide usage
of strlcpy through the AOSP code so it seems inappropriate to start changing
that.

[1] https://lwn.net/Articles/612244/

> 
> which platform is this needed for?

ChromeOS.

https://codereview.chromium.org/1754293002/diff/40001/third_party/libsync/REA...
File third_party/libsync/README.chromium (right):

https://codereview.chromium.org/1754293002/diff/40001/third_party/libsync/REA...
third_party/libsync/README.chromium:6: License: Apache 2.0
On 2016/03/04 at 20:52:04, Nico wrote:
> describe your local changes and how to update this library here

Done.

[xam, 2016-03-16 23:16:28]

lgtm

[Robert Sesek, 2016-03-17 18:40:08]

rsesek@chromium.org changed reviewers:
+ rsesek@chromium.org

[Robert Sesek, 2016-03-17 18:40:08]

LGTM from security

[reveman, 2016-03-17 18:53:44]

thakis, ping for third_party review

[Nico, 2016-03-18 14:48:17]

lgtm

If you need strlcpy to Chrome OS, we can probably get it added to Chrome OS's C
library, right? Seems preferable over each third-party library shipping a copy
:-) (but if this is the only one for now, I suppose as-is is fine too)

[reveman, 2016-03-18 15:49:20]

On 2016/03/18 at 14:48:17, thakis wrote:
> lgtm
> 
> If you need strlcpy to Chrome OS, we can probably get it added to Chrome OS's
C library, right? Seems preferable over each third-party library shipping a copy
:-) (but if this is the only one for now, I suppose as-is is fine too)

That's an option. But we might want to use this on Linux too in the future and
we can't do the same there so I'm going to keep the wrapper here for now.

[reveman, 2016-03-18 15:49:28]

The CQ bit was checked by reveman@chromium.org

[reveman, 2016-03-18 15:49:28]

The patchset sent to the CQ was uploaded after l-g-t-m from piman@chromium.org
Link to the patchset: https://codereview.chromium.org/1754293002/#ps80001
(title: "use strlcpy from aosp")

[commit-bot: I haz the power, 2016-03-18 15:49:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1754293002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1754293002/80001

[commit-bot: I haz the power, 2016-03-18 16:30:55]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-03-18 16:33:12]

Description was changed from

==========
third_party: Add the libsync library from Android

BUG=b/27447486
==========

to

==========
third_party: Add the libsync library from Android

BUG=b/27447486

Committed: https://crrev.com/8d6342181e81b6dd0988283b785d0724c686f096
Cr-Commit-Position: refs/heads/master@{#381989}
==========

[commit-bot: I haz the power, 2016-03-18 16:33:15]

Patchset 5 (id:??) landed as
https://crrev.com/8d6342181e81b6dd0988283b785d0724c686f096
Cr-Commit-Position: refs/heads/master@{#381989}