[PATCH] Removing bad examples of usage of, sysdb_transaction_start/commit/end functions and making it, more consistent. (files src/db/sysdb_*.c)

[Michal Židek]

I worked on solution of ticket https://fedorahosted.org/sssd/ticket/1150 
and I already had
some solutions with macros, but I found them ugly and obfuscated or not 
generic enough, so
they could not be used on all places where we use sysdb_transaction 
functions.

So I decided not to implement any wrapper macros around these functions 
(yet) and only
rewrite the places where we use sysdb_transactions to unify the way they 
are used (and to remove bad
examples of usage).

In this patch, I did it for files in src/db/. If this patch is OK, I'll 
try to spread this pattern to other modules
where sysdb_transaction_start/commit/cancel is used.

The patch is attached.

NOTE: Function sysdb_idmap_store_mappings looks like completely 
rewritten, but it is not. Except of
changes related to sysdb_transaction it had only bad indentation.

Thanks
Michal

[Stephen Gallagher]

On Wed, 2012-08-15 at 17:52 +0200, Michal Židek wrote:
> I worked on solution of ticket https://fedorahosted.org/sssd/ticket/1150 
> and I already had
> some solutions with macros, but I found them ugly and obfuscated or not 
> generic enough, so
> they could not be used on all places where we use sysdb_transaction 
> functions.
>
> So I decided not to implement any wrapper macros around these functions 
> (yet) and only
> rewrite the places where we use sysdb_transactions to unify the way they 
> are used (and to remove bad
> examples of usage).
>
> In this patch, I did it for files in src/db/. If this patch is OK, I'll 
> try to spread this pattern to other modules
> where sysdb_transaction_start/commit/cancel is used.
>
> The patch is attached.
>
> NOTE: Function sysdb_idmap_store_mappings looks like completely 
> rewritten, but it is not. Except of
> changes related to sysdb_transaction it had only bad indentation.

Ack, but in the future please separate unrelated changes into a separate
patch to make review easier. In this particular case, I just regenerated
the patch for review with 'git format-patch -b', but in general it's
better to keep logically-unrelated patches separate.

[Michal Židek]

On 08/15/2012 06:41 PM, Stephen Gallagher wrote:
> On Wed, 2012-08-15 at 17:52 +0200, Michal Židek wrote:
> > I worked on solution of ticket https://fedorahosted.org/sssd/ticket/1150
> > and I already had
> > some solutions with macros, but I found them ugly and obfuscated or not
> > generic enough, so
> > they could not be used on all places where we use sysdb_transaction
> > functions.
> >
> > So I decided not to implement any wrapper macros around these functions
> > (yet) and only
> > rewrite the places where we use sysdb_transactions to unify the way they
> > are used (and to remove bad
> > examples of usage).
> >
> > In this patch, I did it for files in src/db/. If this patch is OK, I'll
> > try to spread this pattern to other modules
> > where sysdb_transaction_start/commit/cancel is used.
> >
> > The patch is attached.
> >
> > NOTE: Function sysdb_idmap_store_mappings looks like completely
> > rewritten, but it is not. Except of
> > changes related to sysdb_transaction it had only bad indentation.
>
> Ack, but in the future please separate unrelated changes into a separate
> patch to make review easier. In this particular case, I just regenerated
> the patch for review with 'git format-patch -b', but in general it's
> better to keep logically-unrelated patches separate.
>
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Just realized, that this was not pushed to master. I rewrote
the commit message (originally this was the first part).

Patch attached.

Thanks
Michal

[Jakub Hrozek]

On Mon, Aug 27, 2012 at 01:03:18PM +0200, Michal Židek wrote:
> On 08/15/2012 06:41 PM, Stephen Gallagher wrote:
> > On Wed, 2012-08-15 at 17:52 +0200, Michal Židek wrote:
> > > I worked on solution of ticket https://fedorahosted.org/sssd/ticket/1150
> > > and I already had
> > > some solutions with macros, but I found them ugly and obfuscated or not
> > > generic enough, so
> > > they could not be used on all places where we use sysdb_transaction
> > > functions.
> > >
> > > So I decided not to implement any wrapper macros around these functions
> > > (yet) and only
> > > rewrite the places where we use sysdb_transactions to unify the way they
> > > are used (and to remove bad
> > > examples of usage).
> > >
> > > In this patch, I did it for files in src/db/. If this patch is OK, I'll
> > > try to spread this pattern to other modules
> > > where sysdb_transaction_start/commit/cancel is used.
> > >
> > > The patch is attached.
> > >
> > > NOTE: Function sysdb_idmap_store_mappings looks like completely
> > > rewritten, but it is not. Except of
> > > changes related to sysdb_transaction it had only bad indentation.
> >
> > Ack, but in the future please separate unrelated changes into a separate
> > patch to make review easier. In this particular case, I just regenerated
> > the patch for review with 'git format-patch -b', but in general it's
> > better to keep logically-unrelated patches separate.
> >
> > sssd-devel mailing list
> > [e-mail]
> > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>
> Just realized, that this was not pushed to master. I rewrote
> the commit message (originally this was the first part).
>
> Patch attached.
>
> Thanks
> Michal

Ack

[Jakub Hrozek]

On Tue, Sep 04, 2012 at 01:55:53PM +0200, Jakub Hrozek wrote:
> On Mon, Aug 27, 2012 at 01:03:18PM +0200, Michal Židek wrote:
> > On 08/15/2012 06:41 PM, Stephen Gallagher wrote:
> > > On Wed, 2012-08-15 at 17:52 +0200, Michal Židek wrote:
> > > > I worked on solution of ticket https://fedorahosted.org/sssd/ticket/1150
> > > > and I already had
> > > > some solutions with macros, but I found them ugly and obfuscated or not
> > > > generic enough, so
> > > > they could not be used on all places where we use sysdb_transaction
> > > > functions.
> > > >
> > > > So I decided not to implement any wrapper macros around these functions
> > > > (yet) and only
> > > > rewrite the places where we use sysdb_transactions to unify the way they
> > > > are used (and to remove bad
> > > > examples of usage).
> > > >
> > > > In this patch, I did it for files in src/db/. If this patch is OK, I'll
> > > > try to spread this pattern to other modules
> > > > where sysdb_transaction_start/commit/cancel is used.
> > > >
> > > > The patch is attached.
> > > >
> > > > NOTE: Function sysdb_idmap_store_mappings looks like completely
> > > > rewritten, but it is not. Except of
> > > > changes related to sysdb_transaction it had only bad indentation.
> > >
> > > Ack, but in the future please separate unrelated changes into a separate
> > > patch to make review easier. In this particular case, I just regenerated
> > > the patch for review with 'git format-patch -b', but in general it's
> > > better to keep logically-unrelated patches separate.
> > >
> > > sssd-devel mailing list
> > > [e-mail]
> > > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> >
> > Just realized, that this was not pushed to master. I rewrote
> > the commit message (originally this was the first part).
> >
> > Patch attached.
> >
> > Thanks
> > Michal
>
> Ack

Pushed to master.