[riot-notifications] [RIOT-OS/RIOT] net/sock_util: Accept NULL pointers in urlsplit (#11677)

Juan I Carrano notifications at github.com
Tue Jun 11 17:59:00 CEST 2019


jcarrano requested changes on this pull request.

Just a couple of details. Otherwise it is OK.

> @@ -116,30 +117,35 @@ static char* _find_pathstart(const char *url)
 
 int sock_urlsplit(const char *url, char *hostport, char *urlpath)
 {
+    assert(url);

```suggestion
    assert(url != NULL);
```

>      char *hoststart = _find_hoststart(url);
     if (!hoststart) {
         return -EINVAL;
     }
 
     char *pathstart = _find_pathstart(hoststart);
 
-    size_t hostlen = pathstart - hoststart;
-    /* hostlen must be smaller SOCK_HOSTPORT_MAXLEN to have space for the null
-     * terminator */
-    if (hostlen > SOCK_HOSTPORT_MAXLEN - 1) {
-        return -EOVERFLOW;
+    if (hostport) {
+        size_t hostlen = pathstart - hoststart;
+        /* hostlen must be smaller SOCK_HOSTPORT_MAXLEN to have space for the null
+        * terminator */
+        if (hostlen > SOCK_HOSTPORT_MAXLEN - 1) {
+            return -EOVERFLOW;
+        }
+        memcpy(hostport, hoststart, hostlen);
+        *(hostport + hostlen) = '\0';

```suggestion
        hostport[hostlen] = '\0';
```
Note: i know you didn't write that, but considering there are not many chances to do little fixes like this...

>      char *hoststart = _find_hoststart(url);
     if (!hoststart) {
         return -EINVAL;
     }
 
     char *pathstart = _find_pathstart(hoststart);
 
-    size_t hostlen = pathstart - hoststart;
-    /* hostlen must be smaller SOCK_HOSTPORT_MAXLEN to have space for the null
-     * terminator */
-    if (hostlen > SOCK_HOSTPORT_MAXLEN - 1) {
-        return -EOVERFLOW;
+    if (hostport) {

```suggestion
    if (hostport != NULL) {
```

>  
-    size_t pathlen = strlen(pathstart);
-    if (pathlen) {
-        if (pathlen > SOCK_URLPATH_MAXLEN - 1) {
-            return -EOVERFLOW;
+    if (urlpath) {

```suggestion
    if (urlpath != NULL) {
```

>  
-    size_t pathlen = strlen(pathstart);
-    if (pathlen) {
-        if (pathlen > SOCK_URLPATH_MAXLEN - 1) {
-            return -EOVERFLOW;
+    if (urlpath) {
+        size_t pathlen = strlen(pathstart);
+        if (pathlen) {

This outer "if" is not necessary.

>          }
-        memcpy(urlpath, pathstart, pathlen);
+        *(urlpath + pathlen) = '\0';

```suggestion
        urlpath[pathlen] = '\0';
```
same as above, since you are you are touching the code, why not fix this too.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/11677#pullrequestreview-248271322
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190611/68e91d21/attachment-0001.html>


More information about the notifications mailing list