From a16d00d673e1acf4c840112dddfaa8ea035bdca3 Mon Sep 17 00:00:00 2001
From: Andrew Gaul <andrew@gaul.org>
Date: Wed, 23 Dec 2020 20:29:33 +0900
Subject: [PATCH] Simply curl_slist_sort_insert (#1494)

Some good taste from Linus:
https://github.com/mkirchner/linked-list-good-taste
Also avoid an allocation when replacing a value and tighten up tests.
---
 src/curl_util.cpp      | 62 ++++++++++++++----------------------------
 src/test_curl_util.cpp |  5 +++-
 2 files changed, 24 insertions(+), 43 deletions(-)

diff --git a/src/curl_util.cpp b/src/curl_util.cpp
index cf8753f..f2c7163 100644
--- a/src/curl_util.cpp
+++ b/src/curl_util.cpp
@@ -55,31 +55,22 @@ struct curl_slist* curl_slist_sort_insert(struct curl_slist* list, const char* d
 
 struct curl_slist* curl_slist_sort_insert(struct curl_slist* list, const char* key, const char* value)
 {
-    struct curl_slist* curpos;
-    struct curl_slist* lastpos;
-    struct curl_slist* new_item;
-
     if(!key){
         return list;
     }
-    if(NULL == (new_item = reinterpret_cast<struct curl_slist*>(malloc(sizeof(struct curl_slist))))){
-        return list;
-    }
 
     // key & value are trimmed and lower (only key)
     std::string strkey = trim(std::string(key));
-    std::string strval = trim(std::string(value ? value : ""));
+    std::string strval = value ? trim(std::string(value)) : "";
     std::string strnew = key + std::string(": ") + strval;
-    if(NULL == (new_item->data = strdup(strnew.c_str()))){
-        free(new_item);
+    char* data;
+    if(NULL == (data = strdup(strnew.c_str()))){
         return list;
     }
-    new_item->next = NULL;
 
-    // cppcheck-suppress unmatchedSuppression
-    // cppcheck-suppress nullPointerRedundantCheck
-    for(lastpos = NULL, curpos = list; curpos; lastpos = curpos, curpos = curpos->next){
-        std::string strcur = curpos->data;
+    struct curl_slist **p = &list;
+    for(;*p; p = &(*p)->next){
+        std::string strcur = (*p)->data;
         size_t pos;
         if(std::string::npos != (pos = strcur.find(':', 0))){
             strcur = strcur.substr(0, pos);
@@ -87,38 +78,25 @@ struct curl_slist* curl_slist_sort_insert(struct curl_slist* list, const char* k
 
         int result = strcasecmp(strkey.c_str(), strcur.c_str());
         if(0 == result){
-            // same data, so replace it.
-            if(lastpos){
-                lastpos->next = new_item;
-            }else{
-                list = new_item;
-            }
-            new_item->next = curpos->next;
-            free(curpos->data);
-            free(curpos);
+            free((*p)->data);
+            (*p)->data = data;
+            return list;
+        }else if(result < 0){
             break;
-
-        }else if(0 > result){
-          // add data before curpos.
-          if(lastpos){
-              lastpos->next = new_item;
-          }else{
-              list = new_item;
-          }
-          new_item->next = curpos;
-          break;
         }
     }
 
-    if(!curpos){
-        // append to last pos
-        if(lastpos){
-            lastpos->next = new_item;
-        }else{
-            // a case of list is null
-            list = new_item;
-        }
+    struct curl_slist* new_item;
+    if(NULL == (new_item = reinterpret_cast<struct curl_slist*>(malloc(sizeof(*new_item))))){
+        free(data);
+        return list;
     }
+
+    struct curl_slist* before = *p;
+    *p = new_item;
+    new_item->data = data;
+    new_item->next = before;
+
     return list;
 }
 
diff --git a/src/test_curl_util.cpp b/src/test_curl_util.cpp
index ca14724..0b31d90 100644
--- a/src/test_curl_util.cpp
+++ b/src/test_curl_util.cpp
@@ -28,7 +28,7 @@
 
 void assert_is_sorted(struct curl_slist* list, const char *file, int line)
 {
-    for(; list != NULL && list->next != NULL; list = list->next){
+    for(; list != NULL; list = list->next){
         std::string key1 = list->data;
         key1.erase(key1.find(':'));
         std::string key2 = list->data;
@@ -40,6 +40,7 @@ void assert_is_sorted(struct curl_slist* list, const char *file, int line)
             std::exit(1);
         }
     }
+    std::cerr << std::endl;
 }
 
 size_t curl_slist_length(const struct curl_slist* list)
@@ -67,11 +68,13 @@ void test_sort_insert()
     // add to head
     list = curl_slist_sort_insert(list, "1", "val");
     ASSERT_IS_SORTED(list);
+    ASSERT_STREQUALS("1: val", list->data);
     // replace head
     list = curl_slist_sort_insert(list, "1", "val2");
     ASSERT_IS_SORTED(list);
     ASSERT_EQUALS(static_cast<size_t>(4), curl_slist_length(list));
     ASSERT_STREQUALS("1: val2", list->data);
+    curl_slist_free_all(list);
 }
 
 int main(int argc, char *argv[])