Pvs analyze part3

[p-alik]

last part of commits cherry-picked from #114

[esabol]

This one surprised me. I actually didn't know you could use a curly brace initializer list with new. Is that really the preferred syntax these days?

I get removing the antiquated check for NULL after calling the constructor in lines 93-96, but is anything set to catch the memory allocation exception if gearmand does run out of memory wherever this object is instantiated? I'm just wondering if we need a try/catch here instead.

[p-alik]

If there is any concern about curly braces I'll replace them. But actually it doesn't matter in this case.

#include <iostream>
#include <string>

class Foo {
    std::string _v{};
    int _a;
    public:
        Foo(std::string v, int a = 0) : _v(v), _a{a} {}
        std::string v(){ return _v; }
        int a() { return _a; }
        
};

int main()
{
    Foo x{"foo"};
    std::cout << "x v: " << x.v() << " a: " << x.a() << std::endl;

    Foo *y = new Foo("bar", 123);
    std::cout << "y v: " << y->v() << " a: " << y->a() << std::endl;

    Foo *z = new Foo{"baz", 321};
    std::cout << "z v: " << z->v()  << " a: " << z->a() << std::endl;

    return 0;
}

$g++ -o main *.cpp -Wall -std=c++0x
$main
x v: foo a: 0
y v: bar a: 123
z v: baz a: 321

[esabol]

I am mainly curious as to the current state of the art of C++ programming. If you say that's the preferred way to instantiate an object with new, then I believe you. I haven't been keeping up with C++ since it's not what I code in on a daily basis.

What I would classify as a concern though is this:

I get removing the antiquated check for NULL after calling the constructor in lines 93-96, but is anything set to catch the memory allocation exception if gearmand does run out of memory wherever this object is instantiated? I'm just wondering if we need a try/catch here instead.

[esabol]

Hello, @p-alik. Please respond to this concern.

[p-alik]

I beg your pardon, @esabol for being irresponsible.
Any objection if we remove f39d20c from the PR?
As to me we shouldn't spend more time on sqlite plugin at all. At least the feature of plugins is decided.

[esabol]

Any objection if we remove f39d20c from the PR?

The fix seems easy to me and worth doing. Why not put try { ... } catch (std::bad_alloc &) { return GEARMAND_MEMORY_ALLOCATION_FAILURE; } around the new?

[p-alik]

It's done in f56b75c

[esabol]

This seems like a significant change. Can you explain?

[p-alik]

It was done in order to fix

Viva64-EM
full
124
/nfs/home/dmka/src/gearmand/libgearman-server/struct/io.h
error
V730
Not all members of a class are initialized inside the constructor. Consider inspecting: options, _state, send_state, recv_state, events, revents, ...
false
3

  gearmand_io_st() { }
------------

see p-alik#8

[illuusio]

This is personal and I know that PVS is good but {} after short and int feels little bit over done why not int something = 0 or some other?

[p-alik]

why not int something = 0 or some other?

for the sake of consistency

[esabol]

LGTM. 👍