Replace macros and fixes memory leak and possible UB

ai/default/aiplayer.cpp

@@ -54,7 +54,8 @@ void dai_player_free(struct ai_type *ait, struct player *pplayer)
 
   if (player_data != nullptr) {
     player_set_ai_data(pplayer, ait, nullptr);
-    FC_FREE(player_data);
+    delete player_data;
+    player_data = nullptr;
   }
 }
 

ai/default/aisettler.cpp

@@ -247,7 +247,7 @@ static void cityresult_destroy(struct cityresult *result)
   if (result != nullptr) {
     if (result->tdc_hash != nullptr) {
       for (const auto *ptdc : qAsConst(*result->tdc_hash)) {
-        NFCPP_FREE(ptdc);
+        delete[] ptdc;
       }
       delete result->tdc_hash;
     }
@@ -379,7 +379,7 @@ static struct cityresult *cityresult_fill(struct ai_type *ait,
     }
 
     if (result->tdc_hash->contains(cindex)) {
-      NFCPP_FREE(result->tdc_hash->value(cindex));
+      delete[] result->tdc_hash->value(cindex);
     }
     result->tdc_hash->insert(cindex, ptdc);
   }
@@ -540,7 +540,7 @@ static void tdc_plr_set(struct ai_type *ait, struct player *plr, int tindex,
 #endif // FREECIV_DEBUG
 
   if (ai->settler->tdc_hash->contains(tindex)) {
-    NFCPP_FREE(ai->settler->tdc_hash->value(tindex));
+    delete[] ai->settler->tdc_hash->value(tindex);
   }
   ai->settler->tdc_hash->insert(tindex, ptdc);
 }
@@ -1196,7 +1196,7 @@ void dai_auto_settler_reset(struct ai_type *ait, struct player *pplayer)
 #endif // FREECIV_DEBUG
 
   for (const auto *ptdc : qAsConst(*ai->settler->tdc_hash)) {
-    NFCPP_FREE(ptdc);
+    delete[] ptdc;
   }
   ai->settler->tdc_hash->clear();
 
@@ -1213,7 +1213,7 @@ void dai_auto_settler_free(struct ai_plr *ai)
   fc_assert_ret(ai != nullptr);
 
   if (ai->settler) {
-    NFC_FREE(ai->settler->tdc_hash);
+    delete ai->settler->tdc_hash;
     delete[] ai->settler;
   }
   ai->settler = nullptr;

ai/default/aiunit.cpp

@@ -3260,7 +3260,7 @@ void dai_unit_init(struct ai_type *ait, struct unit *punit)
   /* Make sure that contents of unit_ai structure are correctly initialized,
    * if you ever allocate it by some other mean than new */
 
-  struct unit_ai *unit_data = new unit_ai[1]();
+  struct unit_ai *unit_data = new unit_ai{};
 
   unit_data->done = false;
   unit_data->cur_pos = nullptr;
@@ -3301,7 +3301,8 @@ void dai_unit_close(struct ai_type *ait, struct unit *punit)
 
   if (unit_data != nullptr) {
     unit_set_ai_data(punit, ait, nullptr);
-    FCPP_FREE(unit_data);
+    delete unit_data;
+    unit_data = nullptr;
   }
 }
 

ai/default/daicity.cpp

@@ -1154,7 +1154,8 @@ void dai_city_free(struct ai_type *ait, struct city *pcity)
   if (city_data != nullptr) {
     adv_deinit_choice(&(city_data->choice));
     city_set_ai_data(pcity, ait, nullptr);
-    FCPP_FREE(city_data);
+    delete[] city_data;
+    city_data = nullptr;
   }
 }
 

ai/tex/texai.c

@@ -70,7 +70,8 @@ static void texai_module_close(void)
 {
   TEXAI_AIT;
 
-  FC_FREE(ait->private);
+  delete ait->private;
+  ait->private = nullptr;
 }
 
 /**********************************************************************/ /**
@@ -568,8 +569,6 @@ const char *fc_ai_tex_capstr(void) { return FC_AI_MOD_CAPSTR; }
  **************************************************************************/
 bool fc_ai_tex_setup(struct ai_type *ai)
 {
-  struct dai_private_data *private;
-
   if (!has_thread_cond_impl()) {
     qCritical(_("This Freeciv21 compilation has no full threads "
                 "implementation, tex ai cannot be used."));
@@ -578,12 +577,10 @@ bool fc_ai_tex_setup(struct ai_type *ai)
 
   qstrncpy(ai->name, "tex", sizeof(ai->name));
 
-private
-  = fc_malloc(sizeof(struct dai_private_data));
-private
-  ->contemplace_workers =
+  ai->private = new struct dai_private_data {
+  };
+  ai->private->contemplace_workers =
       false; /* We use custom code to set worker want and type */
-  ai->private = private;
 
   texai_init_self(ai);
 

ai/tex/texaicity.c

@@ -561,7 +561,7 @@ void texai_req_worker_task_rcv(struct texai_req *req)
     struct worker_task *ptask = worker_task_list_get(pcity->task_reqs, 0);
 
     if (ptask == NULL) {
-      ptask = fc_malloc(sizeof(struct worker_task));
+      ptask = new worker_task{};
       worker_task_init(ptask);
       worker_task_list_append(pcity->task_reqs, ptask);
     }
@@ -585,7 +585,8 @@ void texai_req_worker_task_rcv(struct texai_req *req)
  **************************************************************************/
 void texai_city_alloc(struct ai_type *ait, struct city *pcity)
 {
-  struct texai_city *city_data = fc_calloc(1, sizeof(struct texai_city));
+  struct texai_city *city_data = new struct texai_city {
+  };
 
   city_data->defai.building_wait = BUILDING_WAIT_MINIMUM;
   adv_init_choice(&(city_data->defai.choice));
@@ -602,7 +603,8 @@ void texai_city_free(struct ai_type *ait, struct city *pcity)
 
   if (city_data != NULL) {
     adv_deinit_choice(&(city_data->defai.choice));
-    city_set_ai_data(pcity, ait, NULL);
-    FC_FREE(city_data);
+    city_set_ai_data(pcity, ait, nullptr);
+    delete city_data;
+    city_data = nullptr;
   }
 }

ai/tex/texaiplayer.c

@@ -251,7 +251,8 @@ static enum texai_abort_msg_class texai_check_messages(struct ai_type *ait)
       ret_abort = new_abort;
     }
 
-    FC_FREE(msg);
+    delete msg;
+    msg = nullptr;
 
     texaimsg_list_allocate_mutex(exthrai.msgs_to.msglist);
   }
@@ -265,7 +266,7 @@ static enum texai_abort_msg_class texai_check_messages(struct ai_type *ait)
  **************************************************************************/
 void texai_player_alloc(struct ai_type *ait, struct player *pplayer)
 {
-  struct texai_plr *player_data = fc_calloc(1, sizeof(struct texai_plr));
+  struct texai_plr *player_data = new texai_plr{};
 
   player_set_ai_data(pplayer, ait, player_data);
 
@@ -288,7 +289,8 @@ void texai_player_free(struct ai_type *ait, struct player *pplayer)
   if (player_data != NULL) {
     player_set_ai_data(pplayer, ait, NULL);
     unit_list_destroy(player_data->units);
-    FC_FREE(player_data);
+    delete player_data;
+    player_data = nullptr;
   }
 }
 
@@ -378,15 +380,17 @@ void texai_refresh(struct ai_type *ait, struct player *pplayer)
         if (pcity != NULL && city_owner(pcity) == req->plr) {
           adv_choice_copy(&(def_ai_city_data(pcity, ait)->choice),
                           &(choice_req->choice));
-          FC_FREE(choice_req);
+          delete choice_req;
+          choice_req = nullptr;
         }
       } break;
       case TEXAI_REQ_TURN_DONE:
         req->plr->ai_phase_done = true;
         break;
       }
 
-      FC_FREE(req);
+      delete req;
+      req = nullptr;
 
       texaireq_list_allocate_mutex(exthrai.reqs_from.reqlist);
     }

ai/threaded/taicity.c

@@ -502,7 +502,7 @@ void tai_req_worker_task_rcv(struct tai_req *req)
     struct worker_task *ptask = worker_task_list_get(pcity->task_reqs, 0);
 
     if (ptask == NULL) {
-      ptask = fc_malloc(sizeof(struct worker_task));
+      ptask = new worker_task{};
       worker_task_init(ptask);
       worker_task_list_append(pcity->task_reqs, ptask);
     }

ai/threaded/taiplayer.c

@@ -153,7 +153,8 @@ static enum tai_abort_msg_class tai_check_messages(struct ai_type *ait)
       ret_abort = new_abort;
     }
 
-    FC_FREE(msg);
+    delete msg;
+    msg = nullptr;
 
     thrai.msgs_to.msglist.lock();
   }
@@ -167,7 +168,7 @@ static enum tai_abort_msg_class tai_check_messages(struct ai_type *ait)
  **************************************************************************/
 void tai_player_alloc(struct ai_type *ait, struct player *pplayer)
 {
-  struct tai_plr *player_data = fc_calloc(1, sizeof(struct tai_plr));
+  struct tai_plr *player_data = new tai_plr{};
 
   player_set_ai_data(pplayer, ait, player_data);
 
@@ -187,7 +188,8 @@ void tai_player_free(struct ai_type *ait, struct player *pplayer)
 
   if (player_data != NULL) {
     player_set_ai_data(pplayer, ait, NULL);
-    FC_FREE(player_data);
+    delete player_data;
+    player_data = nullptr;
   }
 }
 
@@ -259,7 +261,8 @@ void tai_refresh(struct ai_type *ait, struct player *pplayer)
         break;
       }
 
-      FC_FREE(req);
+      delete req;
+      req = nullptr;
 
       thrai.reqs_from.reqlist.lock();
     }

ai/threaded/threadedai.c

@@ -69,7 +69,8 @@ static void tai_module_close(void)
 {
   TAI_AIT;
 
-  FC_FREE(ait->private);
+  delete ait->private;
+  ait->private = nullptr;
 }
 
 /**********************************************************************/ /**
@@ -563,8 +564,6 @@ const char *fc_ai_threaded_capstr(void) { return FC_AI_MOD_CAPSTR; }
  **************************************************************************/
 bool fc_ai_threaded_setup(struct ai_type *ai)
 {
-  struct dai_private_data *private;
-
   if (!has_thread_cond_impl()) {
     qCritical(_("This Freeciv21 compilation has no full threads "
                 "implementation, threaded ai cannot be used."));
@@ -573,11 +572,9 @@ bool fc_ai_threaded_setup(struct ai_type *ai)
 
   qstrncpy(ai->name, "threaded", sizeof(ai->name));
 
-private
-  = fc_malloc(sizeof(struct dai_private_data));
-private
-  ->contemplace_workers = true;
-  ai->private = private;
+  ai->private = new struct dai_private_data {
+  };
+  ai->private->contemplace_workers = true;
 
   tai_init_self(ai);
 

client/attribute.cpp

@@ -301,11 +301,12 @@ void attribute_flush()
   }
 
   if (pplayer->attribute_block.data) {
-    free(pplayer->attribute_block.data);
+    delete[] pplayer->attribute_block.data;
     pplayer->attribute_block.data = nullptr;
   }
 
-  serialize_hash(attribute_hash, &pplayer->attribute_block.data,
+  serialize_hash(attribute_hash,
+                 reinterpret_cast<void **>(&pplayer->attribute_block.data),
                  &pplayer->attribute_block.length);
   send_attribute_block(pplayer, &client.conn);
 }

client/citydlg_common.cpp

@@ -512,7 +512,8 @@ static void fc__attribute((__format__(__printf__, 5, 6)))
   fc_vsnprintf(buf + qstrlen(buf), bufsz - qstrlen(buf), totalfmt, args);
   va_end(args);
 
-  FC_FREE(sum);
+  delete sum;
+  sum = nullptr;
 }
 
 /**

client/governor.cpp

@@ -100,7 +100,11 @@ class cma_yoloswag {
 Q_GLOBAL_STATIC(cma_yoloswag, gimb)
 
 // deletes governor
-void governor::drop() { NFCN_FREE(m_instance); }
+void governor::drop()
+{
+  delete m_instance;
+  m_instance = nullptr;
+}
 
 governor::~governor() = default;
 

client/gui-qt/chatline.cpp

@@ -224,7 +224,7 @@ void chat_input::send()
 void chat_input::chat_word_list_changed(const QStringList &word_list)
 {
   cmplt = completer();
-  NFC_FREE(cmplt);
+  delete cmplt;
   cmplt = new QCompleter(word_list);
   cmplt->setCaseSensitivity(Qt::CaseInsensitive);
   cmplt->setCompletionMode(QCompleter::InlineCompletion);

client/gui-qt/citydlg.cpp

@@ -107,7 +107,7 @@ progress_bar::progress_bar(QWidget *parent) : QProgressBar(parent)
  */
 progress_bar::~progress_bar()
 {
-  NFC_FREE(pix);
+  delete pix;
   delete sfont;
 }
 
@@ -137,7 +137,7 @@ void progress_bar::set_pixmap(struct universal *target)
   } else {
     sprite = get_building_sprite(tileset, target->value.building);
   }
-  NFC_FREE(pix);
+  delete pix;
   if (sprite == nullptr) {
     pix = nullptr;
     return;
@@ -160,7 +160,7 @@ void progress_bar::set_pixmap(int n)
   if (valid_advance_by_number(n)) {
     sprite = get_tech_sprite(tileset, n);
   }
-  NFC_FREE(pix);
+  delete pix;
   if (sprite == nullptr) {
     pix = nullptr;
     return;
@@ -1522,7 +1522,7 @@ void city_dialog::update_prod_buttons()
  */
 city_dialog::~city_dialog()
 {
-  NFC_FREE(citizen_pixmap);
+  delete citizen_pixmap;
   removeEventFilter(this);
 }
 
@@ -3148,7 +3148,7 @@ production_item::production_item(struct universal *ptarget, QObject *parent)
 production_item::~production_item()
 {
   // allocated as renegade in model
-  NFC_FREE(target);
+  delete target;
 }
 
 /**

client/gui-qt/dialogs.cpp

@@ -829,7 +829,11 @@ qdef_act *qdef_act::action()
 /**
    Deletes qdef_act instance
  */
-void qdef_act::drop() { NFCN_FREE(m_instance); }
+void qdef_act::drop()
+{
+  delete m_instance;
+  m_instance = nullptr;
+}
 
 /**
    Sets default action vs city
@@ -1653,7 +1657,8 @@ static void diplomat_queue_handle_primary(int actor_unit_id)
       fc_assert_action(actor_unit->client.act_prob_cache,
                        client_unit_init_act_prob_cache(actor_unit));
 
-      FC_FREE(actor_unit->client.act_prob_cache);
+      delete[] actor_unit->client.act_prob_cache;
+      actor_unit->client.act_prob_cache = nullptr;
     }
 
     // The action selection process is over, at least for now.