From 5d83790fb256eb80359f354854fb553e36252272 Mon Sep 17 00:00:00 2001
From: Martin Quinson <martin.quinson@loria.fr>
Date: Wed, 7 Sep 2016 23:22:47 +0200
Subject: [PATCH] quickly review the code of Fred, as requested. Nice job, pal

---
 include/simgrid/jedule/jedule_events.hpp   | 2 +-
 include/simgrid/jedule/jedule_platform.hpp | 9 ++++++---
 src/instr/jedule/jedule_events.cpp         | 1 +
 src/instr/jedule/jedule_output.cpp         | 2 --
 src/instr/jedule/jedule_platform.cpp       | 8 ++------
 5 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/include/simgrid/jedule/jedule_events.hpp b/include/simgrid/jedule/jedule_events.hpp
index 1ccee5c803..1a4cd5ae70 100644
--- a/include/simgrid/jedule/jedule_events.hpp
+++ b/include/simgrid/jedule/jedule_events.hpp
@@ -21,7 +21,7 @@ namespace jedule{
 
 XBT_PUBLIC_CLASS Event{
   private:
-    ~Event()=default;
+    virtual ~Event()=default;
   public:
     Event(std::string name, double start_time, double end_time, std::string type)
       : name(name), start_time(start_time), end_time(end_time), type(type){}
diff --git a/include/simgrid/jedule/jedule_platform.hpp b/include/simgrid/jedule/jedule_platform.hpp
index 618f9e0c39..2c06a7524d 100644
--- a/include/simgrid/jedule/jedule_platform.hpp
+++ b/include/simgrid/jedule/jedule_platform.hpp
@@ -22,14 +22,14 @@ namespace jedule{
 XBT_PUBLIC_CLASS Container {
 public:
   Container(std::string name);
-  ~Container()=default;
+  virtual ~Container()=default;
 private:
   int last_id;
-  int is_lowest;
+  int is_lowest = 0;
 public:
   std::string name;
   std::unordered_map<const char*, int> name2id;
-  Container *parent;
+  Container *parent = nullptr;
   std::vector<Container*> children;
   std::vector<sg_host_t> resource_list;
   void addChild(Container* child);
@@ -46,6 +46,7 @@ public:
 SG_BEGIN_DECL()
 typedef simgrid::jedule::Container * jed_container_t;
 
+/* FIXME: jedule_container should be objectified too */
 /** selection of a subset of resources from the original set */
 struct jed_res_subset {
   jed_container_t parent;
@@ -55,6 +56,8 @@ struct jed_res_subset {
 
 typedef struct jed_res_subset s_jed_res_subset_t, *jed_res_subset_t;
 
+/* FIXME: jedule should be objectified too */
+
 typedef struct jedule_struct {
   jed_container_t root_container;
   std::unordered_map<char*, char*> jedule_meta_info;
diff --git a/src/instr/jedule/jedule_events.cpp b/src/instr/jedule/jedule_events.cpp
index 93f494cc00..4303c36d78 100644
--- a/src/instr/jedule/jedule_events.cpp
+++ b/src/instr/jedule/jedule_events.cpp
@@ -16,6 +16,7 @@
 namespace simgrid{
 namespace jedule{
 
+  /* FIXME: this should could maybe be merged into the destructor? */
 void Event::deleteEvent(){
   while (!this->resource_subsets.empty()){
     xbt_free(this->resource_subsets.back());
diff --git a/src/instr/jedule/jedule_output.cpp b/src/instr/jedule/jedule_output.cpp
index 986762b3e3..99a71dc7e5 100644
--- a/src/instr/jedule/jedule_output.cpp
+++ b/src/instr/jedule/jedule_output.cpp
@@ -16,8 +16,6 @@
 
 #if HAVE_JEDULE
 
-#define STR_BUF_SIZE 1024
-
 XBT_LOG_NEW_DEFAULT_SUBCATEGORY(jed_out, jedule, "Logging specific to Jedule output");
 
 xbt_dynar_t jedule_event_list;
diff --git a/src/instr/jedule/jedule_platform.cpp b/src/instr/jedule/jedule_platform.cpp
index d04c00f9f3..a4373b033d 100644
--- a/src/instr/jedule/jedule_platform.cpp
+++ b/src/instr/jedule/jedule_platform.cpp
@@ -28,9 +28,6 @@ namespace jedule {
 Container::Container(std::string name)
 : name(name) {
 
-  this->is_lowest = 0;
-  this->parent = nullptr;
-
   xbt_dict_set(container_name2container, this->name.c_str(), this, nullptr);
 }
 
@@ -136,12 +133,10 @@ std::string Container::getHierarchyAsString(){
 }
 
 void Container::printResources(FILE * jed_file){
-  unsigned int res_nb;
   unsigned int i=0;
   xbt_assert(!this->resource_list.empty());
 
-  res_nb = this->resource_list.size();
-
+  unsigned int res_nb = this->resource_list.size();
   std::string resid = this->getHierarchyAsString();
 
   fprintf(jed_file, "      <rset id=\"%s\" nb=\"%u\" names=\"", resid.c_str(), res_nb);
@@ -176,6 +171,7 @@ static int compare_ids(const void *num1, const void *num2) {
   return *((int*) num1) - *((int*) num2);
 }
 
+/* FIXME: That should be the destructor, shouldnt it? */
 static void jed_free_container(jed_container_t container) {
   if(!container->children.empty()) {
     for (auto child: container->children){
-- 
2.20.1