Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #64 +/- ##
==========================================
- Coverage 30.35% 29.77% -0.58%
==========================================
Files 20 20
Lines 1960 1998 +38
Branches 1422 1445 +23
==========================================
Hits 595 595
- Misses 106 157 +51
+ Partials 1259 1246 -13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
src/attribute.cpp
Outdated
| valueType = ValueType::Encrypted; | ||
| else if (type == "octet") | ||
| valueType = ValueType::Bytes; | ||
| else if (type == "vsa") |
There was a problem hiding this comment.
You can skip this check because you have identical exceptions here and in the else branch.
| for (const auto& t : tok) | ||
| bytes.push_back(std::stoul(t)); | ||
| return new Bytes(code, bytes); | ||
| } |
There was a problem hiding this comment.
There is no default variant. What will happen if the type is ChapPassword or VendorSpecific?
There was a problem hiding this comment.
Or if the type is neither of the enum values?
include/radproto/error.h
Outdated
| suchAttributeCodeAlreadyExists, | ||
| suchAttributeNameWithAnotherTypeAlreadyExists | ||
| suchAttributeNameWithAnotherTypeAlreadyExists, | ||
| typeIsNotSupported |
message function.
include/radproto/error.h
Outdated
| invalidAttributeCode, | ||
| invalidAttributeSize, | ||
| invalidAttributeType, | ||
| invalidValueTypeMember, |
src/attribute.cpp
Outdated
| using Attribute = RadProto::Attribute; | ||
| using ValueType = RadProto::Attribute::ValueType; | ||
|
|
||
| ValueType parseValueType(const std::string& type) |
There was a problem hiding this comment.
Where is the declaration of this function? If it is local - move it to the anonymous namespace. If it is public - put a declaration in the header.
include/radproto/attribute.h
Outdated
| virtual std::vector<uint8_t> toVector(const std::string& secret, const std::array<uint8_t, 16>& auth) const = 0; | ||
| virtual Attribute* clone() const = 0; | ||
|
|
||
| enum class ValueType : uint8_t |
There was a problem hiding this comment.
I don't see how this type is used in public interfaces. If it is only used internally - move it to the cpp-file.
src/attribute.cpp
Outdated
| else if (type == "vsa") | ||
| throw RadProto::Exception(RadProto::Error::typeIsNotSupported); | ||
| else | ||
| throw RadProto::Exception(RadProto::Error::invalidAttributeType); |
There was a problem hiding this comment.
I think, the wording here should be invalidValueType, right?
changed in parseValueType function. Variant default changed in switch in make function.
|
It would be nice to have tests for new function. |
case ValueType::IpAddress and case ValueType Bytes changed in the function make.
2 participants
The make function added to class Attribute.