C + + memory leak combat

The beginning of the development of C + + projects has been 2 months, from the beginning to write the code will not open the memory leak this point. Found a time dedicated to a special event, from the beginning of the dozens of memory leak point to now slowly have some experience, now summed up.

Detection Tool memory leak point practice 0x01 0x02 0x03

Detection Tools

I usually use the following command to start the program, and then trigger the various interfaces once, let the code cover a full point, and then CTRL + C end the program, open the Leak.log file to see the memory leak.

Valgrind--tool=memcheck--leak-check=full--show-reachable=yes--log-file=leak.log <bin file >

When you open the Leak.log file, you will skip to the last leak SUMMARY:

The definitely lost column above shows a leak point that leaks 56bytes of memory. At present we are only concerned about definitely lost leaks, if repaired, leak summary should look like the following:

If found definitely lost leak point, I will use the definitely lost keyword to search for specific leaks, this place needs to be reminded, as far as possible, you rely on the static library plus-g parameters, otherwise can not show the static library leakage point line number, to check the problem is inconvenient. memory leak Point Practice

From my recent several memory leak points explained in detail. 0x01

To judge a condition in a condition of failure

        Redisreply *reply = NULL;
        int Redis_ret;
            for (int i = 0; i < Valid_cmd_num ++i) {Redis_ret = Redisgetreply (Conn->_ctx, (void * *) &reply); if (REDIS_OK!= redis_ret) {ds_log_warning ("Execute CMD by pipeline failed.") ERRNO:[%D], err_msg:[%s] cmd:[%s] ", Conn->_ctx->err, Conn->_ctx->errstr, Cmds[c
                Md_index[i]].c_str ());
            Break
            } std::vector <std::string> Rider_info; if (NULL!= reply && Redis_reply_array = = Reply->type) {for (int j = 0; J < Reply->eleme Nts ++J) {if (redis_reply_string!= reply->element[j]->type) {RIDER_INFO.P
                    Ush_back ("");
                    else {rider_info.push_back (REPLY-&GT;ELEMENT[J]-&GT;STR);
        } if (!rider_info.empty ()) {            Value.push_back (Rider_info);
                } freereplyobject (Reply);
            Reply = NULL;
        } if (NULL!= reply) {freereplyobject (reply);
 }

This method is Redis to execute the pipeline method, I put together the Get/set method, focus on the memory leak point when only taking into account the get case, only got have returned, and the return object is array will call the following method to release the reply pointer, But I ignored the set case Reply->type is not array, so I can not go to the following method, the reply pointer generated during the loop can not be freed, the last for the release of the loop outside the reply can only operate the last reply pointer, This results in a memory leak.

Freereplyobject (reply);
Reply = NULL;

Solving method

In the For loop, as long as the reply!=null to release the pointer, the adjusted code is as follows:

        Redisreply *reply = NULL;
        int Redis_ret;
            for (int i = 0; i < Valid_cmd_num ++i) {Redis_ret = Redisgetreply (Conn->_ctx, (void * *) &reply); if (REDIS_OK!= redis_ret) {ds_log_warning ("Execute CMD by pipeline failed.") ERRNO:[%D], err_msg:[%s] cmd:[%s] ", Conn->_ctx->err, Conn->_ctx->errstr, Cmds[c
                Md_index[i]].c_str ());
            Break
            } std::vector <std::string> Rider_info; if (NULL!= reply) {if (Redis_reply_array = = Reply->type) {for (int j = 0; J < reply->elements; ++J) {if (redis_reply_string!= reply->element[j]->type) {Ride
                        R_info.push_back ("");
                        else {rider_info.push_back (REPLY-&GT;ELEMENT[J]-&GT;STR);
            }
                    }        if (!rider_info.empty ()) {value.push_back (rider_info);
                } freereplyobject (Reply);
                Reply = NULL;
            Ds_log_debug ("test[%d]", 0);
            } if (NULL!= reply) {freereplyobject (reply);
            Ds_log_debug ("test[%d]", 1);
        Reply = NULL;

 }

0x02

Improper position of pointer initialization

    int a::b (FileInfo *file) {if (null = = file) {ds_log_warning ("FileInfo is NULL");
        return-1;
        //Delay 5s load uint64_t start = Commonutil::get_ms_timestamp ();

        avgconf *new_avg_data = new avgconf ();
        Load file std::string file_path = File->_file_path + file->_file_name;
        Std::ifstream In_file (File_path.c_str ());
            if (!in_file) {ds_log_warning ("Open file [%s] failed", FILE_PATH.C_STR ());
            Std::map<std::string, avgconf *>::iterator iter = _avg_info_map.find (File->_file_path);
            if (ITER!= _avg_info_map.end ()) {_avg_info_map.erase (File->_file_path);
            } file->_last_modify_time = File->file_time ();
        return-1;
        } if (!file->is_channged ()) {return 0; }//To New_avg_data set worth the process ... file->_last_modify_time = File->file_time ();
        In_file.clear ();

        In_file.close ();
            if (0 = data_count) {ds_log_warning ("New Avg_data is empty");
            Delete New_avg_data;
            New_avg_data = NULL;
        return 0;
        Std::map<std::string, avgconf *>::iterator iter = _avg_info_map.find (File->_file_path);
            if (ITER!= _avg_info_map.end ()) {//pointer switch, need to release before pointer avgconf *tmp = iter->second;
            Iter->second = New_avg_data;
                if (NULL!= tmp) {//Delay 20s Delete sleep (20);
                Delete tmp;
            TMP = NULL;
            } else {ds_log_debug ("Avg_key: [%s]", FILE-&GT;_FILE_PATH.C_STR ());
        _avg_info_map.insert (Std::make_pair (File->_file_path, new_avg_data));
    return 0;
 }

This is a method that is called every time in a thread, using to determine whether the file has an update, if there is an update to update the memory of the data, no updates are returned directly. The first time the method was invoked, it was fine, but the following pointer was found to be initialized before it was judged:

avgconf *new_avg_data = new avgconf ();

If the method is called after the first time, if the file is not updated, go to the method of judging whether the file is changed, it will be directly return-1, this time the pointer is where the Meng force, no one cares about him.

Solving method

The process of initializing the pointer is followed by the file->is_channged (), which is modified as follows:

 int a::b (FileInfo *file) {if (null = = file) {ds_log_warning ("FileInfo is NULL");
        return-1;
        //Delay 5s load uint64_t start = Commonutil::get_ms_timestamp ();
        Load file std::string file_path = File->_file_path + file->_file_name;
        Std::ifstream In_file (File_path.c_str ());
            if (!in_file) {ds_log_warning ("Open file [%s] failed", FILE_PATH.C_STR ());
            Std::map<std::string, avgconf *>::iterator iter = _avg_info_map.find (File->_file_path);
            if (ITER!= _avg_info_map.end ()) {_avg_info_map.erase (File->_file_path);
            } file->_last_modify_time = File->file_time ();
        return-1;
        } if (!file->is_channged ()) {return 0;
        } avgconf *new_avg_data = new avgconf ();
  It is worth the process to set new_avg_data ... file->_last_modify_time = File->file_time ();      In_file.clear ();

        In_file.close ();
            if (0 = data_count) {ds_log_warning ("New Avg_data is empty");
            Delete New_avg_data;
            New_avg_data = NULL;
        return 0;
        Std::map<std::string, avgconf *>::iterator iter = _avg_info_map.find (File->_file_path);
            if (ITER!= _avg_info_map.end ()) {//pointer switch, need to release before pointer avgconf *tmp = iter->second;
            Iter->second = New_avg_data;
                if (NULL!= tmp) {//Delay 20s Delete sleep (20);
                Delete tmp;
            TMP = NULL;
            } else {ds_log_debug ("Avg_key: [%s]", FILE-&GT;_FILE_PATH.C_STR ());
        _avg_info_map.insert (Std::make_pair (File->_file_path, new_avg_data));
    return 0; }

0x03

Pointer swap, the original pointer is not processed

int a::b (FileInfo *file) {if (null = = file) {ds_log_warning ("FileInfo is NULL");
        return-1;


        //Delay 5s load uint64_t start = Commonutil::get_ms_timestamp ();
        Load file std::string file_path = File->_file_path + file->_file_name;
        Std::ifstream In_file (File_path.c_str ());
            if (!in_file) {ds_log_warning ("Open file [%s] failed", FILE_PATH.C_STR ());
            Std::map<std::string, avgconf *>::iterator iter = _avg_info_map.find (File->_file_path);
            if (ITER!= _avg_info_map.end ()) {_avg_info_map.erase (File->_file_path);
            } file->_last_modify_time = File->file_time ();
        return-1;
        } if (!file->is_channged ()) {return 0;
        } avgconf *new_avg_data = new avgconf ();
 It is worth the process to set new_avg_data ... file->_last_modify_time = File->file_time ();       In_file.clear ();

        In_file.close ();
            if (0 = data_count) {ds_log_warning ("New Avg_data is empty");
            Delete New_avg_data;
            New_avg_data = NULL;
        return 0;
        Std::map<std::string, avgconf *>::iterator iter = _avg_info_map.find (File->_file_path);
        if (ITER!= _avg_info_map.end ()) {iter->second = New_avg_data;
            else {ds_log_debug ("Avg_key: [%s]", FILE-&GT;_FILE_PATH.C_STR ());
        _avg_info_map.insert (Std::make_pair (File->_file_path, new_avg_data));
    return 0;
 }

The problem above is Iter->second = New_avg_data when replacing in-memory data, the original pointer does not perform the action to release the pointer, and then Valgrind checks to avgconf *new_avg_data = new avgconf (); Statement does not release memory.

Solving method

Swap the pointer, store the old pointer with a temporary pointer, and then release.

            avgconf *tmp = iter->second;
            Iter->second = New_avg_data;
            if (NULL!= tmp) {
                //delay 20s delete sleep
                ;
                Delete tmp;
                TMP = NULL;
            }