Copilot commented on code in PR #12856:
URL: https://github.com/apache/trafficserver/pull/12856#discussion_r2761458374
##########
include/iocore/hostdb/HostDBProcessor.h:
##########
@@ -312,11 +313,13 @@ class HostDBRecord : public RefCountObj
* @param query_name Name of the query for the record.
* @param rr_count Number of info instances.
* @param srv_name_size Storage for SRV names, if any.
+ * @param port Port Number, if any.
+ *
* @return An instance sufficient to hold the specified data.
*
* The query name will stored and initialized, and the info instances
initialized.
*/
- static self_type *alloc(swoc::TextView query_name, unsigned rr_count, size_t
srv_name_size = 0);
+ static self_type *alloc(swoc::TextView query_name, unsigned rr_count, size_t
srv_name_size = 0, int port = 0);
Review Comment:
Type inconsistency in parameter: The port parameter is declared as int, but
should be in_port_t or uint16_t to match the type used in HostDBHash (which is
in_port_t) and to correctly represent port numbers which are unsigned 16-bit
values.
```suggestion
static self_type *alloc(swoc::TextView query_name, unsigned rr_count,
size_t srv_name_size = 0, in_port_t port = 0);
```
##########
include/iocore/hostdb/HostDBProcessor.h:
##########
@@ -519,6 +527,10 @@ class HostDBRecord : public RefCountObj
unsigned failed_p : 1; ///< DNS error.
} f;
} flags{0};
+
+private:
+ /// Port Number if hash key includes it.
+ int _port = 0;
Review Comment:
Type inconsistency: The _port field is declared as int, but the source value
hash.port is of type in_port_t (which is typically uint16_t). This should be
changed to in_port_t or uint16_t to match the source type and avoid potential
issues with sign extension. Port numbers are unsigned and range from 0-65535.
```suggestion
uint16_t _port = 0;
```
##########
include/iocore/hostdb/HostDBProcessor.h:
##########
@@ -733,6 +745,12 @@ HostDBRecord::name_view() const
return {this->name(), swoc::TextView::npos};
}
+inline int
+HostDBRecord::port() const
Review Comment:
Type inconsistency in return type: The port() method returns int, but the
underlying _port field should be in_port_t or uint16_t to correctly represent
port numbers. The return type should match the actual type of the port field.
```suggestion
inline auto
HostDBRecord::port() const -> decltype(_port)
```
##########
src/iocore/hostdb/HostDB.cc:
##########
@@ -919,10 +919,11 @@ HostDBContinuation::dnsEvent(int event, HostEnt *e)
// In the event that the lookup failed (SOA response-- for example) we
want to use hash.host_name, since it'll be ""
TextView query_name = (failed || !hash.host_name.empty()) ? hash.host_name
: TextView{e->ent.h_name, strlen(e->ent.h_name)};
- HostDBRecord::Handle r{HostDBRecord::alloc(query_name, valid_records,
failed ? 0 : e->srv_hosts.srv_hosts_length)};
+ HostDBRecord::Handle r{HostDBRecord::alloc(query_name, valid_records,
failed ? 0 : e->srv_hosts.srv_hosts_length, hash.port)};
r->key = hash.hash.fold(); // always set the key
r->af_family = af;
r->flags.f.failed_p = failed;
+ r->_port = hash.port;
Review Comment:
Redundant assignment of _port. The port is already set in the alloc()
function at line 922 (which calls the implementation at line 1558), so this
assignment is unnecessary and should be removed.
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
