New issue
Advanced search Search tips

Issue 674658 link

Starred by 0 users

Issue metadata

Status: Archived
Owner:
Closed: Dec 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

libcros_config: do not use -fvisibility=default

Project Member Reported by vapier@chromium.org, Dec 15 2016

Issue description

the current gyp file is setting -fvisibility=default.  no code should be doing this.  if you need to export symbols, use brillo/brillo_export.h and its defines instead.

this should be done before any consumers of the library show up and start relying on unintended symbols.
 

Comment 1 by sjg@chromium.org, Dec 16 2016

Status: Assigned (was: Unconfirmed)

Comment 2 by sjg@chromium.org, Dec 16 2016

Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 26 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/950f71654d931ea3dfdd077e50d8e65337ac7e01

commit 950f71654d931ea3dfdd077e50d8e65337ac7e01
Author: Simon Glass <sjg@chromium.org>
Date: Fri Dec 16 19:54:56 2016

chromeos-config: Use BRILLO_EXPORT instead of -fvisibility=default

Apparently we should use the former instead of the latter. I've elected to
export the class since there doesn't seem to be any benefit to invidually
exporting the methods.

BUG= chromium:674658 
TEST=FEATURES=test emerge-reef -q --nodeps chromeos-config-tools
Signed-off-by: Simon Glass <sjg@chromium.org>

Change-Id: I6eb015633017deec67223d8cdecf189fee3c54f8
Reviewed-on: https://chromium-review.googlesource.com/421177
Commit-Ready: Simon Glass <sjg@chromium.org>
Tested-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/950f71654d931ea3dfdd077e50d8e65337ac7e01/chromeos-config/libcros_config/fake_cros_config.h
[modify] https://crrev.com/950f71654d931ea3dfdd077e50d8e65337ac7e01/chromeos-config/chromeos-config.gyp
[modify] https://crrev.com/950f71654d931ea3dfdd077e50d8e65337ac7e01/chromeos-config/libcros_config/cros_config.h

Comment 4 by sjg@chromium.org, Dec 26 2016

Status: Fixed (was: Started)

Comment 5 by dchan@google.com, Mar 4 2017

Labels: VerifyIn-58

Comment 6 by dchan@google.com, Apr 17 2017

Labels: VerifyIn-59

Comment 7 by dchan@google.com, May 30 2017

Labels: VerifyIn-60

Comment 8 by dchan@chromium.org, Aug 1 2017

Labels: VerifyIn-61

Comment 9 by dchan@chromium.org, Oct 14 2017

Status: Archived (was: Fixed)

Sign in to add a comment