2013-12-13 4 views
0

Я изо всех сил пытаюсь написать небольшой класс дескриптора файла в C++ 11. Я знаю, что в STL уже есть много возможностей для обработки файлов, но для учебных целей я хотел сделать это сам.Утечка памяти с использованием исключений C++

К сожалению, я, кажется, не понимает, какой эффект исключение имеет на поведении утечки памяти в C++ программы, потому что Valgrind говорит мне, есть 2 утечка памяти в следующем коде:

file.h

#ifndef FILE_H 
#define FILE_H 

#include <iostream> 
#include <memory> 
#include <string> 

#include <stdio.h> 

class FileDeleter { 
public: 
    void operator()(FILE *p); 
}; 

class File { 
public: 
    File(const std::string path); 

private: 
    const std::string _path; 
    std::unique_ptr<FILE, FileDeleter> _fp; 

}; 

#endif // FILE_H 

file.cpp

#include <cstring> 
#include <iostream> 
#include <stdexcept> 
#include <utility> 

#include <stdio.h> 

#include "file.h" 

void FileDeleter::operator()(FILE *p) 
{ 
if (!p) 
    return; 

    if (fclose(p) == EOF) 
     std::cerr << "FileDeleter: Couldn't close file" << std::endl; 
} 

File::File(const std::string path) 
    : _path(std::move(path)), 
     _fp(fopen(_path.c_str(), "a+")) 
{ 
    if (!_fp) 
     throw std::runtime_error("Couldn't open file"); 
} 

main.cpp

#include <cstdlib> 
#include "file.h" 

int main() 
{ 
    File my_file("/root/.bashrc"); 

    return EXIT_SUCCESS; 
} 

Я действительно хотел открыть /root/.bashrc, чтобы на самом деле заставить File ctor выбрасывать исключение. Если я не брошу туда, Вальгринд совершенно счастлив. Что мне здесь не хватает в использовании исключений? Как реализовать простой дескриптор файла «правильно» (исключение безопасно)?

Заранее благодарен!

Примечание: операции чтения/записи по-прежнему отсутствуют, так как я уже борюсь с основами.

Edit # 1: Это фактический выход Valgrind, используя --leak-чек = полный:

==7998== HEAP SUMMARY: 
==7998==  in use at exit: 233 bytes in 3 blocks 
==7998== total heap usage: 5 allocs, 2 frees, 817 bytes allocated 
==7998== 
==7998== 38 bytes in 1 blocks are possibly lost in loss record 1 of 3 
==7998== at 0x4C27CC2: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) 
==7998== by 0x4EEC4F8: std::string::_Rep::_S_create(unsigned long, unsigned long, std::allocator<char> const&) (in /usr/lib/libstdc++.so.6.0.18) 
==7998== by 0x4EEDC30: char* std::string::_S_construct<char const*>(char const*, char const*, std::allocator<char> const&, std::forward_iterator_tag) (in /usr/lib/libstdc++.so.6.0.18) 
==7998== by 0x4EEE047: std::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&) (in /usr/lib/libstdc++.so.6.0.18) 
==7998== by 0x40137D: main (in ../a.out) 
==7998== 
==7998== 43 bytes in 1 blocks are possibly lost in loss record 2 of 3 
==7998== at 0x4C27CC2: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) 
==7998== by 0x4EEC4F8: std::string::_Rep::_S_create(unsigned long, unsigned long, std::allocator<char> const&) (in /usr/lib/libstdc++.so.6.0.18) 
==7998== by 0x4EEDC30: char* std::string::_S_construct<char const*>(char const*, char const*, std::allocator<char> const&, std::forward_iterator_tag) (in /usr/lib/libstdc++.so.6.0.18) 
==7998== by 0x4EEE047: std::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&) (in /usr/lib/libstdc++.so.6.0.18) 
==7998== by 0x400EBF: File::File(std::string) (in /home/frank/3Other/Code/Laboratory/c++/c++namedpipe/a.out) 
==7998== by 0x401390: main (in ../a.out) 
==7998== 
==7998== 152 bytes in 1 blocks are possibly lost in loss record 3 of 3 
==7998== at 0x4C27730: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) 
==7998== by 0x4E8F8F2: __cxa_allocate_exception (in /usr/lib/libstdc++.so.6.0.18) 
==7998== by 0x400E9B: File::File(std::string) (in /home/frank/3Other/Code/Laboratory/c++/c++namedpipe/a.out) 
==7998== by 0x401390: main (in ../a.out) 
==7998== 
==7998== LEAK SUMMARY: 
==7998== definitely lost: 0 bytes in 0 blocks 
==7998== indirectly lost: 0 bytes in 0 blocks 
==7998==  possibly lost: 233 bytes in 3 blocks 
==7998== still reachable: 0 bytes in 0 blocks 
==7998==   suppressed: 0 bytes in 0 blocks 

Edit # 2: Fixed исключение брошено в Destructor.

Редактировать # 3: Удалено класс FileException, вместо этого используется std :: runtime_error.

Редактировать # 4: Добавлено NULL check in deleter.

+3

'FileDeleter :: operator()' вызывается из деструктора, в то время как генерируется исключение. Не делайте другого исключения. Кроме того, какие ошибки или утечки вам показала valgrind? – Useless

+0

Один из способов - предоставить необязательную функцию-член бросания и сделать деструктор молча. – chris

+1

У вас возникла ошибка. Измените 'FileException :: _ what' из ссылки на простое значение (и, используя C++ 11, также измените конструктор, чтобы вместо значения ссылаться на значение, и« переместить »аргумент в элемент). – syam

ответ

2

Я вижу следующие проблемы:

  1. fclose(NULL) не допускается ...
    • , но, к счастью, std::unique_ptr не вызывает Deleter если get() == nullptr, так что это должно быть хорошо здесь
  2. FileDeleter:operator() вызывается из деструктора, то есть он никогда не должен бросать. В вашем конкретном случае любая ошибка, возвращаемая из fclose, приведет к тому, что std::terminate будет называться
  3. ваше исключение должно хранить строку по значению, а не ссылку. Эта ссылка относится к временному, который будет уничтожен к моменту звонка what().
    • As Vlad & Praetorian указал, что вы можете удалить код ошибки, а не фиксировать его, и просто разрешите std::runtime_error обработать это для вас.TBH, если вы не планируете добавлять данные к конкретному файлу в исключение, или вы планируете исключать исключения для файлов отдельно, вам совсем не нужен этот класс.

Edit: с при условии, что выход Valgrind, он выглядит как стек разматывания никогда не заканчивался. Поскольку моя первоначальная мысль о вызове и метании FileDeleter::operator() выглядит неправильно, разумный тест будет следующим: что произойдет, если вы попытаетесь поймать/уловить внутри основного тела?


Общие замечания:

  1. никогда не сгенерирует исключение внутри деструктора
  2. никогда не называют что-то еще, что могло бы бросить, из деструктора

, потому что, если ваш деструктор вызывается во время обработка исключений/разворачивание стека, программа будет немедленно завершена.

+0

Что касается п.3, нет необходимости хранить что-либо, поскольку ['runtime_error'] (http://en.cppreference.com/w/cpp/error/runtime_error) уже делает это (правильный путь). –

+0

'unique_ptr' не будет вызывать удаление, если принадлежащий ему указатель равен 'nullptr', поэтому нет необходимости в проверке NULL. – Praetorian

+0

Действительно ли fclose (NULL) недействителен? На странице руководства говорится, что он устанавливает errno с EBADF, если fp недействителен. – user2036087

Смежные вопросы