-
Notifications
You must be signed in to change notification settings - Fork 40
fix: [network] add network full support #555
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
add network full support pick from: linuxdeepin@1c81683 Log: add network full support Bug: https://pms.uniontech.com/bug-view-286267.html
Reviewer's GuideRefactors network device generation to centralize validation of logical names and MAC addresses, rely on /sys/class/net existence checks, and enhance merging of lshw/hwinfo data so that additional wired/wireless interfaces are discovered and represented correctly. Sequence diagram for updated network device generationsequenceDiagram
participant DeviceGenerator
participant LshwResult
participant HwinfoResult
participant SysClassNet
participant DeviceNetwork
DeviceGenerator->>LshwResult: read lshwNetworkList
loop for each lshwEntry
LshwResult-->>DeviceGenerator: logicalNameLshw, macAddressLshw
DeviceGenerator->>DeviceGenerator: isValidLogicalName(logicalNameLshw)
alt logicalName invalid
DeviceGenerator-->>DeviceGenerator: skip lshwEntry
else logicalName valid
DeviceGenerator->>DeviceGenerator: isValidMAC(macAddressLshw)
alt MAC invalid
DeviceGenerator-->>DeviceGenerator: skip lshwEntry
else MAC valid
alt logicalNameLshw contains wlan and hasWlan
DeviceGenerator-->>DeviceGenerator: skip extra wlan
else first wlan or wired
DeviceGenerator->>DeviceNetwork: new DeviceNetwork
DeviceGenerator->>DeviceNetwork: setInfoFromLshw(lshwEntry)
DeviceGenerator->>DeviceGenerator: lstDevice.append(device)
alt logicalNameLshw contains wlan
DeviceGenerator-->>DeviceGenerator: hasWlan = true
end
end
end
end
end
DeviceGenerator->>HwinfoResult: read hwinfoNetworkList
loop for each hwinfoEntry
HwinfoResult-->>DeviceGenerator: permanentHwAddress, hwAddress, deviceFile
DeviceGenerator-->>DeviceGenerator: macHwinfo = permanentHwAddress if not empty else hwAddress
DeviceGenerator-->>DeviceGenerator: hasMatchLogicalName = false
loop for each device in lstDevice
DeviceNetwork-->>DeviceGenerator: logicalNameLst, hwAddress
alt logicalNameHwinfo is empty or contains p2p
DeviceGenerator->>DeviceNetwork: setEnableValue(false)
else macHwinfo == hwAddress or logicalNameHwinfo == logicalNameLst
DeviceGenerator->>DeviceNetwork: setInfoFromHwinfo(hwinfoEntry)
DeviceGenerator-->>DeviceGenerator: hasMatchLogicalName = true
else no match
DeviceGenerator->>DeviceNetwork: setCanUninstall(false)
DeviceGenerator->>DeviceNetwork: setForcedDisplay(true)
end
end
alt not hasMatchLogicalName
DeviceGenerator->>DeviceGenerator: isValidMAC(macHwinfo)
DeviceGenerator->>DeviceGenerator: isValidLogicalName(logicalNameHwinfo)
alt MAC and logicalName valid
alt logicalNameHwinfo does not contain wlan
DeviceGenerator->>DeviceNetwork: new DeviceNetwork
DeviceGenerator->>DeviceNetwork: setInfoFromHwinfo(hwinfoEntry)
DeviceGenerator->>DeviceGenerator: lstDevice.append(device)
DeviceGenerator-->>DeviceGenerator: hasMatchLogicalName = true
else logicalNameHwinfo contains wlan
alt hasWlan is false
DeviceGenerator->>DeviceNetwork: new DeviceNetwork
DeviceGenerator->>DeviceNetwork: setInfoFromHwinfo(hwinfoEntry)
DeviceGenerator->>DeviceGenerator: lstDevice.append(device)
DeviceGenerator-->>DeviceGenerator: hasMatchLogicalName = true
DeviceGenerator-->>DeviceGenerator: hasWlan = true
else hasWlan is true
DeviceGenerator-->>DeviceGenerator: skip extra wlan
end
end
else invalid MAC or logicalName
DeviceGenerator-->>DeviceGenerator: skip hwinfoEntry
end
end
end
DeviceGenerator->>DeviceNetwork: finalize lstDevice for UI/output
Class diagram for DeviceGenerator network support changesclassDiagram
class DeviceGenerator {
+generatorMonitorDevice() void
+generatorNetworkDevice() void
}
class DeviceNetwork {
+setInfoFromLshw(lshwEntry) void
+setInfoFromHwinfo(hwinfoEntry) void
+setEnableValue(enable) void
+setCanUninstall(canUninstall) void
+setForcedDisplay(forcedDisplay) void
+hwAddress() QString
+logicalName() QString
}
class NetworkValidation {
+isValidLogicalName(logicalName QString) bool
+isValidMAC(macAddress QString) bool
}
DeviceGenerator --> DeviceNetwork : creates_and_updates
DeviceGenerator ..> NetworkValidation : uses
NetworkValidation <.. DeviceNetwork : validated_by
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review我来对这段代码进行审查,并提出改进建议:
以下是改进后的代码建议: bool DeviceGenerator::isValidLogicalName(const QString& logicalName)
{
// 安全检查:防止路径遍历
if (logicalName.contains("..") || logicalName.contains('/') || logicalName.contains('\\')) {
qCWarning(appLog) << "Invalid logical name:" << logicalName;
return false;
}
// 检查是否为p2p设备
if (logicalName.contains("p2p", Qt::CaseInsensitive)) {
return false;
}
// 使用QFileInfo替代QDir提高性能
QString addressFilePath = "/sys/class/net/" + logicalName;
if (!QFileInfo::exists(addressFilePath)) {
qCWarning(appLog) << "Network interface path not exist:" << addressFilePath;
return false;
}
return true;
}
bool DeviceGenerator::isValidMAC(const QString& macAddress)
{
// 更严格的MAC地址验证
if (macAddress.isEmpty() ||
macAddress.compare("00:00:00:00:00:00", Qt::CaseInsensitive) == 0 ||
macAddress.compare("ff:ff:ff:ff:ff:ff", Qt::CaseInsensitive) == 0) {
return false;
}
// 更严格的MAC地址正则表达式
static const QRegularExpression macRegex("^([0-9A-Fa-f]{2}[:-]){5}([0-9A-Fa-f]{2})$");
return macRegex.match(macAddress).hasMatch();
}其他建议:
这些改进将使代码更加健壮、安全和可维护。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- Consider moving
isValidLogicalNameandisValidMACinto an anonymous namespace or making themstaticto limit their visibility to this translation unit and avoid potential name clashes. - In
isValidLogicalName, the log statement prints theQDirobject instead of the actual path string; loggingdir.path()(or the constructed path) would make the message clearer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider moving `isValidLogicalName` and `isValidMAC` into an anonymous namespace or making them `static` to limit their visibility to this translation unit and avoid potential name clashes.
- In `isValidLogicalName`, the log statement prints the `QDir` object instead of the actual path string; logging `dir.path()` (or the constructed path) would make the message clearer.
## Individual Comments
### Comment 1
<location> `deepin-devicemanager/src/GenerateDevice/DeviceGenerator.cpp:297-303` </location>
<code_context>
qCDebug(appLog) << "DeviceGenerator::generatorMonitorDevice end";
}
+bool isValidLogicalName(const QString& logicalName)
+{
+ if (logicalName.contains("p2p", Qt::CaseInsensitive))
+ return false;
+
+ QString addressFilePath = "/sys/class/net/" + logicalName;
+ QDir dir(addressFilePath);
+ if (dir.exists())
+ return true;
</code_context>
<issue_to_address>
**issue (bug_risk):** Empty logical names are treated as valid due to `/sys/class/net/` existing.
When `logicalName` is empty (which can happen in the hwinfo path), `addressFilePath` becomes `/sys/class/net/`, and `QDir("/sys/class/net/").exists()` will almost always be true. As a result, `isValidLogicalName("")` returns `true`, which conflicts with the later intent to only add devices when the logical name exists and is valid, and can result in phantom devices with no logical name. Add an explicit guard such as `if (logicalName.isEmpty()) return false;` before constructing `addressFilePath`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: add-uos, lzwind The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/merge |
add network full support
pick from: 1c81683
Log: add network full support
Bug: https://pms.uniontech.com/bug-view-286267.html
Summary by Sourcery
Improve network device generation to better validate and discover network interfaces from system information.
New Features:
Bug Fixes:
Enhancements: