Merge branch 'maint'

* maint:
  Fix crash when clearing last botset entry for a bot.
  set_set: Remove redunant freeing of newxk when no old and empty new
  set_set: Always need to remove the old entry.
  set_set: If old==new then there's no need to do anything

doc/UPDATES.md

@@ -10,6 +10,7 @@
 
 # maint
   * Clear FiSH keys when a client quits.
+  * Fix crash when clearing last botset entry for a bot.
 
 # 1.4.9
   * Fix various compile warnings and spam

src/mod/share.mod/share.cc

@@ -655,7 +655,7 @@ share_change(int idx, char *par)
         return;
       }
 
-      if (uet->got_share) {
+      if (uet->got_share && uet != &USERENTRY_SET) {
         if (!(e = find_user_entry(uet, u))) {
           e = (struct user_entry *) calloc(1, sizeof(struct user_entry));
 
@@ -669,6 +669,14 @@ share_change(int idx, char *par)
           list_delete((struct list_type **) &(u->entries), (struct list_type *) e);
           free(e);
         }
+      } else if (uet == &USERENTRY_SET) {
+        /*
+         * set_set() chains down to set_user() which mimics
+         * the above and leads to deleting/freeing 'e' when
+         * already done in set_user(). set_gotshare() ignores
+         * e.
+         */
+        uet->got_share(u, NULL, par, idx);
       }
       noshare = 0;
     }

src/userent.cc

@@ -252,6 +252,10 @@ static bool set_set(struct userrec *u, struct user_entry *e, void *buf)
       break;
     }
   }
+
+  /* Nothing to do if the old and new entry match. Can this even happen? */
+  if (old == newxk)
+    return 1;
   
   /* we will possibly free new below, so let's send the information to the botnet now */
   if (!noshare && !set_noshare) {
@@ -264,17 +268,8 @@ static bool set_set(struct userrec *u, struct user_entry *e, void *buf)
     }
   }
 
-  /* unset and bail out if the new data is empty and the old doesn't exist, why'd we even get this change? */
-  if (!old && (!newxk->data || !newxk->data[0])) {
-    /* or simply ... delete non-existant entry */
-    free(newxk->key);
-    free(newxk->data);
-    free(newxk);
-    return 1;
-  }
-
   /* if we have a new entry and an old entry.. or our new entry is empty -> clear out the old entry */
-  if (old && old != newxk && (!newxk->data || !newxk->data[0])) {
+  if (old) {
     list_delete((struct list_type **) (&e->u.extra), (struct list_type *) old);
 
     free(old->key);
@@ -284,7 +279,7 @@ static bool set_set(struct userrec *u, struct user_entry *e, void *buf)
   }
 
   /* add the new entry if it's not empty */
-  if ((!old || old != newxk) && newxk->data && newxk->data[0]) {
+  if (newxk->data && newxk->data[0]) {
     list_insert((&e->u.xk), newxk);
   } else {
     free(newxk->data);
@@ -341,6 +336,8 @@ static bool set_gotshare(struct userrec *u, struct user_entry *e, char *buf, int
 {
   char *name = newsplit(&buf);
 
+  ASSERT(e == NULL, "set_gotshare should not be passed a user_entry");
+
   if (!name || !name[0])
     return 1;