On Thu, Oct 22, 2020 at 13:44:10 +0200, Bjoern Walk wrote:
> Peter Krempa <pkrempa@xxxxxxxxxx> [2020-10-22, 11:34AM +0200]:
> > Rewrite using GHashTable which already has interfaces for using a number
> > as hash key.
> >
> > Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>
> > ---
> > src/conf/domain_addr.c | 123 +++++++++--------------------------------
> > src/conf/domain_addr.h | 4 +-
> > 2 files changed, 27 insertions(+), 100 deletions(-)
[...]
> > @@ -123,40 +122,19 @@ virDomainZPCIAddressAssignFid(virHashTablePtr set,
> >
> >
> > static void
> > -virDomainZPCIAddressReleaseId(virHashTablePtr set,
> > - virZPCIDeviceAddressID *id,
> > - const char *name)
> > +virDomainZPCIAddressReleaseId(GHashTable *set,
> > + virZPCIDeviceAddressID *id)
> > {
> > if (!id->isSet)
> > return;
> >
> > - if (virHashRemoveEntry(set, &id->value) < 0) {
> > - virReportError(VIR_ERR_INTERNAL_ERROR,
> > - _("Release %s %o failed"),
> > - name, id->value);
> > - }
> > + g_hash_table_remove(set, &id->value);
>
> Here I'm not so sure, IIUC the original code reported an error when the
> value was not found in the hash table. This will be dropped with the new
> code. But since this should only occur in pathological cases anyways, so
Specifically, here the error is only logged, but the caller doesn't take
any action/return any value.
That means that there wouldn't be any immediate notification of the
user. It's very unlikely that the error will be noticed in logs.
g_hash_table_remove returns whether it released the value or not, but I
don't feel it's worth keeping it.
> I guess this is fine.
Thanks!
