Skip to content

Commit

Permalink
Add a lint for toplevel "using namespace" in headers.
Browse files Browse the repository at this point in the history
We had all sorts of things using the wrong namespaces because everything was
being imported into multiple namespaces if things happened to include certain
headers.  Remove the "using namespace" bits in core headers, fix the resulting
compile issues, add a lint so people stop doing that.
  • Loading branch information
bzbarsky-apple committed Mar 27, 2024
1 parent 86af239 commit 8191d6e
Show file tree
Hide file tree
Showing 38 changed files with 124 additions and 102 deletions.
15 changes: 15 additions & 0 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -285,3 +285,18 @@ jobs:
if: always()
run: |
git grep -I -n 'SuccessOrExit([^=)]*(' -- './*' ':(exclude).github/workflows/lint.yml' && exit 1 || exit 0
# git grep exits with 0 if it finds a match, but we want
# to fail (exit nonzero) on match.
- name: Check for use of "using namespace" outside of a class/function in headers.
if: always()
run: |
# Various platforms have `using namespace chip::Ble` in their BLEManager* headers; just exclude those for now.
#
# Exclude platform openiotsdk bits that do this in their persistent storage header.
#
# Also exclude examples (for now) and third_party, which have various instances of this.
#
# Ignore uses of `System::Clock::Literals`, because that's the only way to have things using _ms32 or whatnot
# in a header file.
git grep -I -n -e '^using namespace' --and --not -e 'System::Clock::Literals' -- './**/*.h' ':(exclude)src/platform/*/BLEManager*.h' ':(exclude)src/platform/openiotsdk/KVPsaPsStore.h' ':(exclude)./examples' ':(exclude)./third_party' && exit 1 || exit 0
2 changes: 1 addition & 1 deletion examples/chip-tool/commands/pairing/IssueNOCChainCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class IssueNOCChainCommand : public CHIPCommand

static void OnDeviceNOCChainGeneration(void * context, CHIP_ERROR status, const chip::ByteSpan & noc,
const chip::ByteSpan & icac, const chip::ByteSpan & rcac,
chip::Optional<chip::IdentityProtectionKeySpan> ipk,
chip::Optional<chip::Crypto::IdentityProtectionKeySpan> ipk,
chip::Optional<chip::NodeId> adminSubject)
{
auto command = static_cast<IssueNOCChainCommand *>(context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ class OpenCommissioningWindowCommand : public CHIPCommand
"1 to use Enhanced Commissioning Method.\n 0 to use Basic Commissioning Method.");
AddArgument("window-timeout", 0, UINT16_MAX, &mCommissioningWindowTimeout,
"Time, in seconds, before the commissioning window closes.");
AddArgument("iteration", chip::kSpake2p_Min_PBKDF_Iterations, chip::kSpake2p_Max_PBKDF_Iterations, &mIteration,
"Number of PBKDF iterations to use to derive the verifier. Ignored if 'option' is 0.");
AddArgument("iteration", chip::Crypto::kSpake2p_Min_PBKDF_Iterations, chip::Crypto::kSpake2p_Max_PBKDF_Iterations,
&mIteration, "Number of PBKDF iterations to use to derive the verifier. Ignored if 'option' is 0.");
AddArgument("discriminator", 0, 4096, &mDiscriminator, "Discriminator to use for advertising. Ignored if 'option' is 0.");
AddArgument("timeout", 0, UINT16_MAX, &mTimeout, "Time, in seconds, before this command is considered to have timed out.");
}
Expand Down
5 changes: 3 additions & 2 deletions examples/common/pigweed/rpc_services/Device.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "platform/ConfigurationManager.h"
#include "platform/DiagnosticDataProvider.h"
#include "platform/PlatformManager.h"
#include <crypto/CHIPCryptoPAL.h>
#include <platform/DeviceInstanceInfoProvider.h>
#include <setup_payload/QRCodeSetupPayloadGenerator.h>

Expand Down Expand Up @@ -188,9 +189,9 @@ class CommissionableDataProviderRpcWrapper : public DeviceLayer::CommissionableD
private:
std::optional<uint16_t> mDiscriminatorOverride;
std::optional<uint32_t> mPasscodeOverride;
Spake2pVerifierSerialized mVerifierBuf;
Crypto::Spake2pVerifierSerialized mVerifierBuf;
std::optional<ByteSpan> mVerifierOverride;
uint8_t mSaltBuf[kSpake2p_Max_PBKDF_Salt_Length];
uint8_t mSaltBuf[Crypto::kSpake2p_Max_PBKDF_Salt_Length];
std::optional<ByteSpan> mSaltOverride;
std::optional<uint32_t> mIterationCountOverride;
DeviceLayer::CommissionableDataProvider * mCommissionableDataProvider = nullptr;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ using namespace chip::app;
using namespace chip::app::DataModel;
using namespace chip::app::Clusters;
using namespace chip::app::Clusters::ElectricalPowerMeasurement;
using namespace chip::app::Clusters::ElectricalPowerMeasurement::Attributes;
using namespace chip::app::Clusters::ElectricalPowerMeasurement::Structs;

CHIP_ERROR ElectricalPowerMeasurementInstance::Init()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ using namespace chip::app;
using namespace chip::app::Clusters;
using namespace chip::app::Clusters::AdministratorCommissioning;
using namespace chip::Protocols;
using namespace chip::Crypto;
using chip::Protocols::InteractionModel::Status;

class AdministratorCommissioningAttrAccess : public AttributeAccessInterface
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ namespace app {
namespace Clusters {
namespace DeviceEnergyManagement {

using namespace chip::app::Clusters::DeviceEnergyManagement::Attributes;

class Delegate
{
public:
Expand Down Expand Up @@ -160,24 +158,24 @@ class Delegate

// ------------------------------------------------------------------
// Get attribute methods
virtual ESATypeEnum GetESAType() = 0;
virtual bool GetESACanGenerate() = 0;
virtual ESAStateEnum GetESAState() = 0;
virtual int64_t GetAbsMinPower() = 0;
virtual int64_t GetAbsMaxPower() = 0;
virtual PowerAdjustmentCapability::TypeInfo::Type GetPowerAdjustmentCapability() = 0;
virtual DataModel::Nullable<Structs::ForecastStruct::Type> GetForecast() = 0;
virtual OptOutStateEnum GetOptOutState() = 0;
virtual ESATypeEnum GetESAType() = 0;
virtual bool GetESACanGenerate() = 0;
virtual ESAStateEnum GetESAState() = 0;
virtual int64_t GetAbsMinPower() = 0;
virtual int64_t GetAbsMaxPower() = 0;
virtual Attributes::PowerAdjustmentCapability::TypeInfo::Type GetPowerAdjustmentCapability() = 0;
virtual DataModel::Nullable<Structs::ForecastStruct::Type> GetForecast() = 0;
virtual OptOutStateEnum GetOptOutState() = 0;

// ------------------------------------------------------------------
// Set attribute methods
virtual CHIP_ERROR SetESAType(ESATypeEnum) = 0;
virtual CHIP_ERROR SetESACanGenerate(bool) = 0;
virtual CHIP_ERROR SetESAState(ESAStateEnum) = 0;
virtual CHIP_ERROR SetAbsMinPower(int64_t) = 0;
virtual CHIP_ERROR SetAbsMaxPower(int64_t) = 0;
virtual CHIP_ERROR SetPowerAdjustmentCapability(PowerAdjustmentCapability::TypeInfo::Type) = 0;
virtual CHIP_ERROR SetForecast(DataModel::Nullable<Structs::ForecastStruct::Type>) = 0;
virtual CHIP_ERROR SetESAType(ESATypeEnum) = 0;
virtual CHIP_ERROR SetESACanGenerate(bool) = 0;
virtual CHIP_ERROR SetESAState(ESAStateEnum) = 0;
virtual CHIP_ERROR SetAbsMinPower(int64_t) = 0;
virtual CHIP_ERROR SetAbsMaxPower(int64_t) = 0;
virtual CHIP_ERROR SetPowerAdjustmentCapability(Attributes::PowerAdjustmentCapability::TypeInfo::Type) = 0;
virtual CHIP_ERROR SetForecast(DataModel::Nullable<Structs::ForecastStruct::Type>) = 0;

protected:
EndpointId mEndpointId = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@ namespace app {
namespace Clusters {
namespace ElectricalPowerMeasurement {

using namespace chip::app::Clusters::ElectricalPowerMeasurement::Attributes;
using namespace chip::app::Clusters::ElectricalPowerMeasurement::Structs;

class Delegate
{
public:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ using namespace chip::app;
using namespace chip::app::Clusters;
using namespace chip::app::Clusters::OperationalCredentials;
using namespace chip::Credentials;
using namespace chip::Crypto;
using namespace chip::Protocols::InteractionModel;

namespace {
Expand Down
2 changes: 2 additions & 0 deletions src/app/icd/client/CheckInHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@

#include <protocols/secure_channel/Constants.h>

using namespace chip::Protocols::SecureChannel;

namespace chip {
namespace app {

Expand Down
2 changes: 0 additions & 2 deletions src/app/icd/client/DefaultCheckInDelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@
namespace chip {
namespace app {

using namespace std;

class InteractionModelEngine;

/// Callbacks for check in protocol
Expand Down
2 changes: 1 addition & 1 deletion src/app/icd/client/DefaultICDClientStorage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ CHIP_ERROR DefaultICDClientStorage::DeleteAllEntries(FabricIndex fabricIndex)
}

CHIP_ERROR DefaultICDClientStorage::ProcessCheckInPayload(const ByteSpan & payload, ICDClientInfo & clientInfo,
CounterType & counter)
Protocols::SecureChannel::CounterType & counter)
{
uint8_t appDataBuffer[kAppDataLength];
MutableByteSpan appData(appDataBuffer);
Expand Down
3 changes: 2 additions & 1 deletion src/app/icd/client/DefaultICDClientStorage.h
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,8 @@ class DefaultICDClientStorage : public ICDClientStorage
*/
CHIP_ERROR DeleteAllEntries(FabricIndex fabricIndex);

CHIP_ERROR ProcessCheckInPayload(const ByteSpan & payload, ICDClientInfo & clientInfo, CounterType & counter) override;
CHIP_ERROR ProcessCheckInPayload(const ByteSpan & payload, ICDClientInfo & clientInfo,
Protocols::SecureChannel::CounterType & counter) override;

protected:
enum class ClientInfoTag : uint8_t
Expand Down
4 changes: 2 additions & 2 deletions src/app/icd/client/ICDClientStorage.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
namespace chip {
namespace app {

using namespace Protocols::SecureChannel;
/**
* The ICDClientStorage class is an abstract interface that defines the operations
* for storing, retrieving and deleting ICD client information in persistent storage.
Expand Down Expand Up @@ -81,7 +80,8 @@ class ICDClientStorage
* @param[out] clientInfo retrieved matched clientInfo from storage
* @param[out] counter counter value received in the check-in message
*/
virtual CHIP_ERROR ProcessCheckInPayload(const ByteSpan & payload, ICDClientInfo & clientInfo, CounterType & counter) = 0;
virtual CHIP_ERROR ProcessCheckInPayload(const ByteSpan & payload, ICDClientInfo & clientInfo,
Protocols::SecureChannel::CounterType & counter) = 0;

// 4 bytes for counter + 2 bytes for ActiveModeThreshold
static inline constexpr uint8_t kAppDataLength = 6;
Expand Down
1 change: 1 addition & 0 deletions src/app/server/CommissioningWindowManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@

using namespace chip::app::Clusters;
using namespace chip::System::Clock;
using namespace chip::Crypto;

using AdministratorCommissioning::CommissioningWindowStatusEnum;
using chip::app::DataModel::MakeNullable;
Expand Down
7 changes: 4 additions & 3 deletions src/app/server/CommissioningWindowManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <app/data-model/Nullable.h>
#include <app/server/AppDelegate.h>
#include <app/server/CommissioningModeProvider.h>
#include <crypto/CHIPCryptoPAL.h>
#include <lib/core/CHIPVendorIdentifiers.hpp>
#include <lib/core/ClusterEnums.h>
#include <lib/core/DataModelTypes.h>
Expand Down Expand Up @@ -93,7 +94,7 @@ class CommissioningWindowManager : public Messaging::UnsolicitedMessageHandler,
FabricIndex fabricIndex, VendorId vendorId);

CHIP_ERROR OpenEnhancedCommissioningWindow(System::Clock::Seconds16 commissioningTimeout, uint16_t discriminator,
Spake2pVerifier & verifier, uint32_t iterations, chip::ByteSpan salt,
Crypto::Spake2pVerifier & verifier, uint32_t iterations, chip::ByteSpan salt,
FabricIndex fabricIndex, VendorId vendorId);

void CloseCommissioningWindow();
Expand Down Expand Up @@ -204,7 +205,7 @@ class CommissioningWindowManager : public Messaging::UnsolicitedMessageHandler,
uint8_t mFailedCommissioningAttempts = 0;

bool mUseECM = false;
Spake2pVerifier mECMPASEVerifier;
Crypto::Spake2pVerifier mECMPASEVerifier;
uint16_t mECMDiscriminator = 0;
// mListeningForPASE is true only when we are listening for
// PBKDFParamRequest messages or when we're in the middle of a PASE
Expand All @@ -214,7 +215,7 @@ class CommissioningWindowManager : public Messaging::UnsolicitedMessageHandler,
bool mCommissioningTimeoutTimerArmed = false;
uint32_t mECMIterations = 0;
uint32_t mECMSaltLength = 0;
uint8_t mECMSalt[kSpake2p_Max_PBKDF_Salt_Length];
uint8_t mECMSalt[Crypto::kSpake2p_Max_PBKDF_Salt_Length];

// For tests only, so that we can test the commissioning window timeout
// without having to wait 3 minutes.
Expand Down
8 changes: 5 additions & 3 deletions src/app/tests/TestCommissionManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@

#include <nlunit-test.h>

using namespace chip::Crypto;

using chip::CommissioningWindowAdvertisement;
using chip::CommissioningWindowManager;
using chip::Server;
Expand Down Expand Up @@ -328,9 +330,9 @@ void CheckCommissioningWindowManagerEnhancedWindowTask(intptr_t context)
CHIP_ERROR err = chip::DeviceLayer::GetCommissionableDataProvider()->GetSetupDiscriminator(originDiscriminator);
NL_TEST_ASSERT(suite, err == CHIP_NO_ERROR);
uint16_t newDiscriminator = static_cast<uint16_t>(originDiscriminator + 1);
chip::Spake2pVerifier verifier;
constexpr uint32_t kIterations = chip::kSpake2p_Min_PBKDF_Iterations;
uint8_t salt[chip::kSpake2p_Min_PBKDF_Salt_Length];
Spake2pVerifier verifier;
constexpr uint32_t kIterations = kSpake2p_Min_PBKDF_Iterations;
uint8_t salt[kSpake2p_Min_PBKDF_Salt_Length];
chip::ByteSpan saltData(salt);

NL_TEST_ASSERT(suite, !sWindowStatusDirty);
Expand Down
1 change: 1 addition & 0 deletions src/controller/AutoCommissioner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ namespace chip {
namespace Controller {

using namespace chip::app::Clusters;
using namespace chip::Crypto;
using chip::app::DataModel::MakeNullable;
using chip::app::DataModel::NullNullable;

Expand Down
4 changes: 3 additions & 1 deletion src/controller/AutoCommissioner.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <controller/CommissioneeDeviceProxy.h>
#include <controller/CommissioningDelegate.h>
#include <credentials/DeviceAttestationConstructor.h>
#include <crypto/CHIPCryptoPAL.h>
#include <protocols/secure_channel/RendezvousParameters.h>

namespace chip {
Expand Down Expand Up @@ -70,7 +71,8 @@ class AutoCommissioner : public CommissioningDelegate
ByteSpan GetDAC() const { return ByteSpan(mDAC, mDACLen); }
ByteSpan GetPAI() const { return ByteSpan(mPAI, mPAILen); }

CHIP_ERROR NOCChainGenerated(ByteSpan noc, ByteSpan icac, ByteSpan rcac, IdentityProtectionKeySpan ipk, NodeId adminSubject);
CHIP_ERROR NOCChainGenerated(ByteSpan noc, ByteSpan icac, ByteSpan rcac, Crypto::IdentityProtectionKeySpan ipk,
NodeId adminSubject);
EndpointId GetEndpoint(const CommissioningStage & stage) const;
CommissioningStage GetNextCommissioningStageInternal(CommissioningStage currentStage, CHIP_ERROR & lastErr);

Expand Down
1 change: 1 addition & 0 deletions src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ using namespace chip::System;
using namespace chip::Transport;
using namespace chip::Credentials;
using namespace chip::app::Clusters;
using namespace chip::Crypto;

namespace chip {
namespace Controller {
Expand Down
16 changes: 9 additions & 7 deletions src/controller/CHIPDeviceController.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
#include <credentials/FabricTable.h>
#include <credentials/attestation_verifier/DeviceAttestationDelegate.h>
#include <credentials/attestation_verifier/DeviceAttestationVerifier.h>
#include <crypto/CHIPCryptoPAL.h>
#include <inet/InetInterface.h>
#include <lib/core/CHIPConfig.h>
#include <lib/core/CHIPCore.h>
Expand Down Expand Up @@ -79,8 +80,6 @@ namespace chip {

namespace Controller {

using namespace chip::Protocols::UserDirectedCommissioning;

inline constexpr uint16_t kNumMaxActiveDevices = CHIP_CONFIG_CONTROLLER_MAX_ACTIVE_DEVICES;

struct ControllerInitParams
Expand Down Expand Up @@ -272,7 +271,7 @@ class DLL_EXPORT DeviceController : public AbstractDnssdDiscoveryController
* @return CHIP_ERROR CHIP_NO_ERROR on success, or corresponding error
*/
CHIP_ERROR ComputePASEVerifier(uint32_t iterations, uint32_t setupPincode, const ByteSpan & salt,
Spake2pVerifier & outVerifier);
Crypto::Spake2pVerifier & outVerifier);

void RegisterDeviceDiscoveryDelegate(DeviceDiscoveryDelegate * delegate) { mDeviceDiscoveryDelegate = delegate; }

Expand Down Expand Up @@ -720,7 +719,10 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
* Return the UDC Server instance
*
*/
UserDirectedCommissioningServer * GetUserDirectedCommissioningServer() { return mUdcServer; }
Protocols::UserDirectedCommissioning::UserDirectedCommissioningServer * GetUserDirectedCommissioningServer()
{
return mUdcServer;
}
#endif // CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY

/**
Expand Down Expand Up @@ -785,7 +787,7 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
ObjectPool<CommissioneeDeviceProxy, kNumMaxActiveDevices> mCommissioneeDevicePool;

#if CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY // make this commissioner discoverable
UserDirectedCommissioningServer * mUdcServer = nullptr;
Protocols::UserDirectedCommissioning::UserDirectedCommissioningServer * mUdcServer = nullptr;
// mUdcTransportMgr is for insecure communication (ex. user directed commissioning)
UdcTransportMgr * mUdcTransportMgr = nullptr;
uint16_t mUdcListenPort = CHIP_UDC_PORT;
Expand Down Expand Up @@ -821,7 +823,7 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
The function does not hold a reference to the device object.
*/
CHIP_ERROR SendOperationalCertificate(DeviceProxy * device, const ByteSpan & nocCertBuf, const Optional<ByteSpan> & icaCertBuf,
IdentityProtectionKeySpan ipk, NodeId adminSubject,
Crypto::IdentityProtectionKeySpan ipk, NodeId adminSubject,
Optional<System::Clock::Timeout> timeout);
/* This function sends the trusted root certificate to the device.
The function does not hold a reference to the device object.
Expand Down Expand Up @@ -886,7 +888,7 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
Credentials::AttestationVerificationResult result);

static void OnDeviceNOCChainGeneration(void * context, CHIP_ERROR status, const ByteSpan & noc, const ByteSpan & icac,
const ByteSpan & rcac, Optional<IdentityProtectionKeySpan> ipk,
const ByteSpan & rcac, Optional<Crypto::IdentityProtectionKeySpan> ipk,
Optional<NodeId> adminSubject);
static void OnArmFailSafe(void * context,
const chip::app::Clusters::GeneralCommissioning::Commands::ArmFailSafeResponse::DecodableType & data);
Expand Down
2 changes: 1 addition & 1 deletion src/controller/CHIPDeviceControllerFactory.h
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ struct FactoryInitParams
Inet::EndPointManager<Inet::TCPEndPoint> * tcpEndPointManager = nullptr;
Inet::EndPointManager<Inet::UDPEndPoint> * udpEndPointManager = nullptr;
FabricTable * fabricTable = nullptr;
OperationalKeystore * operationalKeystore = nullptr;
Crypto::OperationalKeystore * operationalKeystore = nullptr;
Credentials::OperationalCertificateStore * opCertStore = nullptr;
SessionResumptionStorage * sessionResumptionStorage = nullptr;
#if CONFIG_NETWORK_LAYER_BLE
Expand Down
Loading

0 comments on commit 8191d6e

Please sign in to comment.