4
4
4ainik2018-10-12 21:10:39
C++ / C#
4ainik, 2018-10-12 21:10:39

What is the error in the source code, and is there one?

Hello. There is a simple source code:

class TLibrary{
    HINSTANCE hModule;
    TLibrary( const TLibrary &){}
    TLibrary(){}
public:
    TLibrary(const char *LibName){
        if( ( hModule = LoadLibrary(LibName) ) == NULL )
            throw Exception("Can't load library");
    }
    virtual FARPROC getProcAddress(const char *ProcName){
        FARPROC ptr = GetProcAddress(hModule, ProcName);
        if( ptr == NULL )
            throw Exception("Can't find func");
        return ptr;
    }
    virtual ~TLibrary(){
        FreeLibrary(hModule);
    }
};

I thought that it does not contain errors, and it actually compiles successfully without any warnings and even works. But a simple check with a static code analyzer revealed certain potential problems:
http://cppcheck.sourceforge.net/cgi-bin/democlient.cgi
Cppcheck 1.84

[test.cpp:3]: (warning, inconclusive) Member variable 'TLibrary::hModule' is not initialized in the constructor.
[test.cpp:6]: (style) Class 'TLibrary' has a constructor with 1 argument that is not explicit. Such constructors should in general be explicit for type safety reasons. Using the explicit keyword in the constructor means some mistakes when using the class can be avoided.
[test.cpp:1]: (warning) The class 'TLibrary' has 'copy constructor' but lack of 'operator='.
Done!

Are there experts here who could explain in simple native language what is the problem here?

Answer the question

In order to leave comments, you need to log in

3 answer(s)
A
Alexey Grichenko, 2018-10-12
@4ainik

I understand that this is C ++ (it would be better to put it in the tags).
Judging by the code, you just want to hide a couple of constructors. To do this, it is enough to declare them in the private part of the class, but it is not necessary to define them. Change it to this and you'll be happy:
TLibrary( const TLibrary &);
TLibrary();

V
Vitaly, 2018-10-12
@vt4a2h

[test.cpp:3]: (warning, inconclusive) Member variable 'TLibrary::hModule' is not initialized in the constructor.

In C++, everything (almost) must be explicitly initialized, otherwise you can get very interesting problems later. In the constructor, you must initialize all the fields of the class in the order of declaration. Starting with C++11, you can default-initialize with equals at the time of declaration.
It is better to add explicit to one-argument constructors to avoid implicit conversions and any interesting effects in the course of the program.
Golden Rule: If you have implemented a destructor, a copy constructor, a copy operator, a move constructor, or a move operator yourself, consider that you may need to implement all five methods.

V
Vasily Melnikov, 2018-10-30
@BacCM

Depends on how this class will be used. But there are potentially errors:
1. When copying a class object, the original object and the copy will have the same hModule value.
And if double freeing can and will work relatively painlessly, then there is no function call for the freed handle.
2. Exception from the constructor. It will not lead to anything bad here, but potentially when expanding the functionality and as an approach in general, it is very bad
3. The fact that the class field can potentially be uninitialized is also understandable.

Didn't find what you were looking for?

Ask your question

Ask a Question

731 491 924 answers to any question