On Sat, Mar 28, 2020 at 5:26 AM Bart Van Assche [off-list ref] wrote:

On 2020-03-20 05:16, Jack Wang wrote:

quoted

+/**
+ * rtrs_str_to_sockaddr() - Convert rtrs address string to sockaddr
+ * @addr:    String representation of an addr (IPv4, IPv6 or IB GID):
+ *              - "ip:192.168.1.1"
+ *              - "ip:fe80::200:5aee:feaa:20a2"
+ *              - "gid:fe80::200:5aee:feaa:20a2"
+ * @len:        String address length
+ * @port:    Destination port
+ * @dst:     Destination sockaddr structure
+ *
+ * Returns 0 if conversion successful. Non-zero on error.
+ */
+static int rtrs_str_to_sockaddr(const char *addr, size_t len,
+                              short port, struct sockaddr_storage *dst)
+{
+     if (strncmp(addr, "gid:", 4) == 0) {
+             return rtrs_str_gid_to_sockaddr(addr + 4, len - 4, port, dst);
+     } else if (strncmp(addr, "ip:", 3) == 0) {
+             char port_str[8];
+             char *cpy;
+             int err;
+
+             snprintf(port_str, sizeof(port_str), "%u", port);
+             cpy = kstrndup(addr + 3, len - 3, GFP_KERNEL);
+             err = cpy ? inet_pton_with_scope(&init_net, AF_UNSPEC,
+                                              cpy, port_str, dst) : -ENOMEM;
+             kfree(cpy);
+
+             return err;
+     }
+     return -EPROTONOSUPPORT;
+}

Please use 'u16' or 'uint16_t' for port numbers instead of 'short'.

ok, will use u16.

quoted

+/**
+ * rtrs_addr_to_sockaddr() - convert path string "src,dst" to sockaddreses
+ * @str:     string containing source and destination addr of a path
+ *           separated by comma. I.e. "ip:1.1.1.1,ip:1.1.1.2". If str
+ *           contains only one address it's considered to be destination.
+ * @len:     string length
+ * @port:    will be set to port.

                ^^^^^^^^^^^^^^^^^^^
What does this mean? Please make comments easy to comprehend.

how about just "port number"?

quoted

+ * @addr:    will be set to the source/destination address or to NULL
+ *           if str doesn't contain any sorce address.

                                           ^^^^^
Is this perhaps a typo?

yes, should be "source".

quoted

+ *
+ * Returns zero if conversion successful. Non-zero otherwise.
+ */
+int rtrs_addr_to_sockaddr(const char *str, size_t len, short port,

                                                          ^^^^^
I think most kernel code uses type u16 for port numbers.

ok.

quoted

+                        struct rtrs_addr *addr)
+{
+     const char *d;
+
+     d = strchr(str, ',');
+     if (!d)
+             d = strchr(str, '@');
+     if (d) {
+             if (rtrs_str_to_sockaddr(str, d - str, 0, addr->src))

                                                      ^^^
Does this mean that the @port argument only applies to the destination
address? If so, please mention this in the comment above this function.

Yes, will update the comments.

quoted

+                     return -EINVAL;
+             d += 1;
+             len -= d - str;
+             str  = d;
+
+     } else {
+             addr->src = NULL;
+     }
+     return rtrs_str_to_sockaddr(str, len, port, addr->dst);
+}
+EXPORT_SYMBOL(rtrs_addr_to_sockaddr);

So this function either accepts ',' or '@' as separator between source
and destination address?  Shouldn't that be mentioned in the comment
block above the function?

Yes, will update the comment.

Thanks,

Bart.

Thanks Bart!