2014-12-18 16 views
6

Mi sono imbattuto in un codice che mi ha sconvolto. Essenzialmente si segue questo schema:C++ Costruisce l'oggetto due volte usando un nuovo comportamento non definito?

class Foo 
{ 
    public: 
    //default constructor 
    Foo(): x(0), ptr(nullptr) 
    { 
     //do nothing 
    } 

    //more interesting constructor 
    Foo(FooInitialiser& init): x(0), ptr(nullptr) 
    { 
     x = init.getX(); 
     ptr = new int; 
    } 
    ~Foo() 
    { 
     delete ptr; 
    } 

    private: 
    int x; 
    int* ptr; 
}; 


void someFunction(FooInitialiser initialiser) 
{ 
    int numFoos = MAGIC_NUMBER; 
    Foo* fooArray = new Foo[numFoos]; //allocate an array of default constructed Foo's 

    for(int i = 0; i < numFoos; ++i) 
    { 
     new(fooArray+ i) Foo(initialiser); //use placement new to initialise 
    } 

    //... do stuff 

    delete[] fooArray; 
} 

Questo codice è stato nella base di codice per anni e sembrerebbe non ha mai causato un problema. È ovviamente una cattiva idea dal momento che qualcuno potrebbe cambiare il costruttore predefinito per allocare non aspettandosi la seconda costruzione. Sostituire semplicemente il secondo costruttore con un metodo di inizializzazione equivalente sembrerebbe la cosa più sensata da fare. per esempio.

void Foo::initialise(FooInitialiser& init) 
{ 
    x = init.getX(); 
    ptr = new int; 
} 

Anche se ancora soggetto a possibili perdite di risorse, almeno un programmatore di difesa potrebbe pensare di controllare per le assegnazioni precedenti in un metodo normale.

La mia domanda è:

sta costruendo due volte come questo comportamento in realtà non identificato/fuori legge da solo una cattiva idea standard o semplicemente? Se un comportamento indefinito puoi citare o indicarmi il posto giusto per cercare nello standard?

+0

hai provato valgrind su questo codice? – zoska

+0

Il problema principale che vedo è che 'Foo' non segue la regola del tre - l'operatore predefinito copy-ctor e copy-assignment-non farà la cosa giusta con' Foo :: ptr'. – cdhowie

+0

@cdhowie Forse non dovremmo assumere il peggio del codice degli altri. Immagino che OP abbia semplicemente ritagliato il codice che non era necessario per porre la domanda. – anatolyg

risposta

10

Generalmente, lavorare con un posizionamento nuovo in questo modo non è una buona idea. La chiamata di un inizializzatore dal primo nuovo o la chiamata di un inizializzatore anziché del nuovo posizionamento sono entrambi considerati una forma migliore del codice che hai fornito.

Tuttavia, in questo caso, il comportamento del posizionamento di chiamata nuovo su un oggetto esistente è ben definito.

Un programma può finire la durata di qualsiasi oggetto riutilizzando stoccaggio che l'oggetto occupa o chiamando esplicitamente il distruttore per un oggetto di un tipo di classe con un distruttore non banale. Per un oggetto di un tipo di classe con un distruttore non banale, il programma non è necessario per chiamare esplicitamente il distruttore prima che l'archivio che occupa l'oggetto sia riutilizzato o rilasciato; tuttavia, se non è presente una chiamata esplicita al distruttore o se un delete-expression (5.3.5) è non utilizzato per rilasciare la memoria, il distruttore non deve essere chiamato implicitamente e qualsiasi programma che dipende dagli effetti collaterali prodotto dal distruttore ha un comportamento non definito.

Così, quando questo accade:

Foo* fooArray = new Foo[numFoos]; //allocate an array of default constructed Foo's 

for(int i = 0; i < numFoos; ++i) 
{ 
    new(fooArray+ i) Foo(initialiser); //use placement new to initialise 
} 

Il collocamento nuova operazione si concluderà il ciclo di vita del Foo che era lì, e crearne uno nuovo al suo posto. In molte circostanze questo potrebbe essere negativo, ma dato il modo in cui funziona il tuo distruttore, questo andrà bene.

Chiamare il nuovo posizionamento su un oggetto esistente potrebbe essere un comportamento non definito, ma dipende dall'oggetto specifico.

Questo non produce un comportamento indefinito, perché non si dipende dagli "effetti collaterali" prodotti dal distruttore.

L'unico "effetto collaterale" nel distruttore del vostro oggetto è quello di delete il int puntatore contenuto, ma in questo caso che l'oggetto non è mai in uno stato cancellabile quando il posizionamento new viene chiamato.

Se fosse possibile per il contenuto int puntatore sia uguale a qualcosa di diverso nullptr e potrebbe richiedere la cancellazione, quindi chiamando il posizionamento new sopra l'oggetto esistente sarebbe invocare un comportamento indefinito.

+3

Sebbene accurato, considererei questo riutilizzo una pessima idea. –

+1

Ci sono molti costrutti simili nella nostra codebase> 30 anni e sono tutti i risultati della transizione incompiuta da c a C++ –

+1

@MooingDuck Agreed. Grazie, ho chiarito che questa è probabilmente una cattiva idea, ma che in questo caso è ben definita. –