From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39189) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gWNIu-0001Tb-Um for qemu-devel@nongnu.org; Mon, 10 Dec 2018 10:17:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gWNIt-0003XT-Ia for qemu-devel@nongnu.org; Mon, 10 Dec 2018 10:17:04 -0500 Date: Mon, 10 Dec 2018 15:16:48 +0000 From: Daniel =?utf-8?B?UC4gQmVycmFuZ8Op?= Message-ID: <20181210151648.GN20272@redhat.com> Reply-To: Daniel =?utf-8?B?UC4gQmVycmFuZ8Op?= References: <20181207161351.4380-1-vsementsov@virtuozzo.com> <20181207161351.4380-6-vsementsov@virtuozzo.com> <20181210145203.GM20272@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3 5/5] crypto: support multiple threads accessing one QCryptoBlock List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alberto Garcia Cc: Vladimir Sementsov-Ogievskiy , qemu-devel@nongnu.org, qemu-block@nongnu.org, mreitz@redhat.com, kwolf@redhat.com, den@openvz.org On Mon, Dec 10, 2018 at 04:09:31PM +0100, Alberto Garcia wrote: > On Mon 10 Dec 2018 03:52:03 PM CET, Daniel P. Berrang=C3=A9 wrote: > >> > +int qcrypto_block_init_cipher(QCryptoBlock *block, > >> > + QCryptoCipherAlgorithm alg, > >> > + QCryptoCipherMode mode, > >> > + const uint8_t *key, size_t nkey, > >> > + size_t n_threads, Error **errp) > >> > +{ > >> > + size_t i; > >> > + > >> > + assert(!block->ciphers && !block->n_ciphers && !block->n_free= _ciphers); > >> > + > >> > + block->ciphers =3D g_new0(QCryptoCipher *, n_threads); > >>=20 > >> You can use g_new() instead of g_new0() because you're anyway > >> overwriting all elements of the array. > > > > I'd rather have it initialized to zero upfront, so if creating any > > cipher in the array fails, we don't have uninitialized array elements > > during later cleanup code. >=20 > But it is the value of block->n_ciphers that determines the size of the > array, and that is only incremented after each successful iteration: >=20 > for (i =3D 0; i < n_threads; i++) { > block->ciphers[i] =3D qcrypto_cipher_new(alg, mode, key, nkey, er= rp); > /* ... error handling ... */ > block->n_ciphers++; > block->n_free_ciphers++; > } >=20 > In other words, the cleanup code won't touch uninitialized elements > because it cannot even tell the difference between an index that points > to an uninitialized element of the array and an index that points beyon= d > the allocated memory. .../correctly written/ cleanup code won't touch uninitialized elements... I prefer to see the code be hardened against mistakes and so would always zero-initialize everything unless there's compelling performance reason not to in certain specific places. Regards, Daniel --=20 |: https://berrange.com -o- https://www.flickr.com/photos/dberran= ge :| |: https://libvirt.org -o- https://fstop138.berrange.c= om :| |: https://entangle-photo.org -o- https://www.instagram.com/dberran= ge :|