Review Request 874: [1/1] Account: Better failure check of lock pool initialization.

[Stephen Gallagher]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-openlmi.rhcloud.com/r/874/
-----------------------------------------------------------

Review request for OpenLMI Developers.


Repository: openlmi-providers


Description
-------

Account: Better failure check of lock pool initialization.


Diffs
-----

  src/account/LMI_AccountProvider.c 5805f5e9637a77585c3eb63e75f8ae2bdfbbf063 

Diff: http://reviewboard-openlmi.rhcloud.com/r/874/diff/


Testing
-------


Thanks,

Robin Hack


_______________________________________________
openlmi-reviews mailing list
openlmi-reviews(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/openlmi-reviews

[Stephen Gallagher]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-openlmi.rhcloud.com/r/874/#review1200
-----------------------------------------------------------

Ship it!


Ship It!


src/account/LMI_AccountProvider.c
<http://reviewboard-openlmi.rhcloud.com/r/874/#comment742>

    The ret variable is not used anywhere, I would remove it


- Roman Rakus


On Sept. 18, 2013, 12:28 p.m., Robin Hack wrote:
>  
>  -----------------------------------------------------------
>  This is an automatically generated e-mail. To reply, visit:
>  http://reviewboard-openlmi.rhcloud.com/r/874/
>  -----------------------------------------------------------
>  
>  (Updated Sept. 18, 2013, 12:28 p.m.)
>  
>  
>  Review request for OpenLMI Developers.
>  
>  
>  Repository: openlmi-providers
>  
>  
>  Description
>  -------
>  
>  Account: Better failure check of lock pool initialization.
>  
>  
>  Diffs
>  -----
>  
>    src/account/LMI_AccountProvider.c 5805f5e9637a77585c3eb63e75f8ae2bdfbbf063 
>  
>  Diff: http://reviewboard-openlmi.rhcloud.com/r/874/diff/
>  
>  
>  Testing
>  -------
>  
>  
>  Thanks,
>  
>  Robin Hack
>  
>  

_______________________________________________
openlmi-reviews mailing list
openlmi-reviews(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/openlmi-reviews

[Stephen Gallagher]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-openlmi.rhcloud.com/r/874/#review1201
-----------------------------------------------------------



src/account/LMI_AccountProvider.c
<http://reviewboard-openlmi.rhcloud.com/r/874/#comment743>

    I just don't see, what's so bad about:
    if (init_lock_pool() == 0) {
    ...
    
    Seems more readable to me.


- Michal Minar


On Sept. 18, 2013, 12:28 p.m., Robin Hack wrote:
>  
>  -----------------------------------------------------------
>  This is an automatically generated e-mail. To reply, visit:
>  http://reviewboard-openlmi.rhcloud.com/r/874/
>  -----------------------------------------------------------
>  
>  (Updated Sept. 18, 2013, 12:28 p.m.)
>  
>  
>  Review request for OpenLMI Developers.
>  
>  
>  Repository: openlmi-providers
>  
>  
>  Description
>  -------
>  
>  Account: Better failure check of lock pool initialization.
>  
>  
>  Diffs
>  -----
>  
>    src/account/LMI_AccountProvider.c 5805f5e9637a77585c3eb63e75f8ae2bdfbbf063 
>  
>  Diff: http://reviewboard-openlmi.rhcloud.com/r/874/diff/
>  
>  
>  Testing
>  -------
>  
>  
>  Thanks,
>  
>  Robin Hack
>  
>  

_______________________________________________
openlmi-reviews mailing list
openlmi-reviews(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/openlmi-reviews

[Stephen Gallagher]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-openlmi.rhcloud.com/r/874/#review1202
-----------------------------------------------------------

Ship it!


Ship It!

- Michal Minar


On Sept. 18, 2013, 12:28 p.m., Robin Hack wrote:
>  
>  -----------------------------------------------------------
>  This is an automatically generated e-mail. To reply, visit:
>  http://reviewboard-openlmi.rhcloud.com/r/874/
>  -----------------------------------------------------------
>  
>  (Updated Sept. 18, 2013, 12:28 p.m.)
>  
>  
>  Review request for OpenLMI Developers.
>  
>  
>  Repository: openlmi-providers
>  
>  
>  Description
>  -------
>  
>  Account: Better failure check of lock pool initialization.
>  
>  
>  Diffs
>  -----
>  
>    src/account/LMI_AccountProvider.c 5805f5e9637a77585c3eb63e75f8ae2bdfbbf063 
>  
>  Diff: http://reviewboard-openlmi.rhcloud.com/r/874/diff/
>  
>  
>  Testing
>  -------
>  
>  
>  Thanks,
>  
>  Robin Hack
>  
>  

_______________________________________________
openlmi-reviews mailing list
openlmi-reviews(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/openlmi-reviews

[Stephen Gallagher]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-openlmi.rhcloud.com/r/874/#review1203
-----------------------------------------------------------



src/account/LMI_AccountProvider.c
<http://reviewboard-openlmi.rhcloud.com/r/874/#comment744>

    This change is totally useless and introduces a variable that is thrown away after a
few lines anyway.


- Jan Synacek


On Sept. 18, 2013, 2:28 p.m., Robin Hack wrote:
>  
>  -----------------------------------------------------------
>  This is an automatically generated e-mail. To reply, visit:
>  http://reviewboard-openlmi.rhcloud.com/r/874/
>  -----------------------------------------------------------
>  
>  (Updated Sept. 18, 2013, 2:28 p.m.)
>  
>  
>  Review request for OpenLMI Developers.
>  
>  
>  Repository: openlmi-providers
>  
>  
>  Description
>  -------
>  
>  Account: Better failure check of lock pool initialization.
>  
>  
>  Diffs
>  -----
>  
>    src/account/LMI_AccountProvider.c 5805f5e9637a77585c3eb63e75f8ae2bdfbbf063 
>  
>  Diff: http://reviewboard-openlmi.rhcloud.com/r/874/diff/
>  
>  
>  Testing
>  -------
>  
>  
>  Thanks,
>  
>  Robin Hack
>  
>  

_______________________________________________
openlmi-reviews mailing list
openlmi-reviews(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/openlmi-reviews

[Stephen Gallagher]

>  On Sept. 18, 2013, 12:40 p.m., Jan Synacek wrote:
>  > src/account/LMI_AccountProvider.c, line 59
>  >
> <http://reviewboard-openlmi.rhcloud.com/r/874/diff/1/?file=4916#file4916li...
>  >
>  >     This change is totally useless and introduces a variable that is thrown away
> after a few lines anyway. 
Ok. GCC without optimalizations convinst me.
With variable:
   0x000000000040047f <+0>:	push   %rbp
   0x0000000000400480 <+1>:	mov    %rsp,%rbp
   0x0000000000400483 <+4>:	sub    $0x10,%rsp
   0x0000000000400487 <+8>:	mov    $0x0,%eax
   0x000000000040048c <+13>:	callq  0x400474 <c>
   0x0000000000400491 <+18>:	mov    %eax,-0x4(%rbp)
   0x0000000000400494 <+21>:	cmpl   $0x1,-0x4(%rbp)
   0x0000000000400498 <+25>:	jne    0x4004a1 <main+34>
   0x000000000040049a <+27>:	mov    $0x0,%eax
   0x000000000040049f <+32>:	jmp    0x4004a6 <main+39>
   0x00000000004004a1 <+34>:	mov    $0x1,%eax
   0x00000000004004a6 <+39>:	leaveq 
   0x00000000004004a7 <+40>:	retq   

Without variable:
   0x000000000040047f <+0>:	push   %rbp
   0x0000000000400480 <+1>:	mov    %rsp,%rbp
   0x0000000000400483 <+4>:	mov    $0x0,%eax
   0x0000000000400488 <+9>:	callq  0x400474 <c>
   0x000000000040048d <+14>:	cmp    $0x1,%eax
   0x0000000000400490 <+17>:	jne    0x400499 <main+26>
   0x0000000000400492 <+19>:	mov    $0x0,%eax
   0x0000000000400497 <+24>:	jmp    0x40049e <main+31>
   0x0000000000400499 <+26>:	mov    $0x1,%eax
   0x000000000040049e <+31>:	leaveq 
   0x000000000040049f <+32>:	retq   


- Robin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-openlmi.rhcloud.com/r/874/#review1203
-----------------------------------------------------------


On Sept. 18, 2013, 12:28 p.m., Robin Hack wrote:
>  
>  -----------------------------------------------------------
>  This is an automatically generated e-mail. To reply, visit:
>  http://reviewboard-openlmi.rhcloud.com/r/874/
>  -----------------------------------------------------------
>  
>  (Updated Sept. 18, 2013, 12:28 p.m.)
>  
>  
>  Review request for OpenLMI Developers.
>  
>  
>  Repository: openlmi-providers
>  
>  
>  Description
>  -------
>  
>  Account: Better failure check of lock pool initialization.
>  
>  
>  Diffs
>  -----
>  
>    src/account/LMI_AccountProvider.c 5805f5e9637a77585c3eb63e75f8ae2bdfbbf063 
>  
>  Diff: http://reviewboard-openlmi.rhcloud.com/r/874/diff/
>  
>  
>  Testing
>  -------
>  
>  
>  Thanks,
>  
>  Robin Hack
>  
>  

_______________________________________________
openlmi-reviews mailing list
openlmi-reviews(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/openlmi-reviews

[Stephen Gallagher]

>  On Sept. 18, 2013, 2:40 p.m., Jan Synacek wrote:
>  > src/account/LMI_AccountProvider.c, line 59
>  >
> <http://reviewboard-openlmi.rhcloud.com/r/874/diff/1/?file=4916#file4916li...
>  >
>  >     This change is totally useless and introduces a variable that is thrown away
> after a few lines anyway.
>  
>  Robin Hack wrote:
>      Ok. GCC without optimalizations convinst me.
>      With variable:
>         0x000000000040047f <+0>:	push   %rbp
>         0x0000000000400480 <+1>:	mov    %rsp,%rbp
>         0x0000000000400483 <+4>:	sub    $0x10,%rsp
>         0x0000000000400487 <+8>:	mov    $0x0,%eax
>         0x000000000040048c <+13>:	callq  0x400474 <c>
>         0x0000000000400491 <+18>:	mov    %eax,-0x4(%rbp)
>         0x0000000000400494 <+21>:	cmpl   $0x1,-0x4(%rbp)
>         0x0000000000400498 <+25>:	jne    0x4004a1 <main+34>
>         0x000000000040049a <+27>:	mov    $0x0,%eax
>         0x000000000040049f <+32>:	jmp    0x4004a6 <main+39>
>         0x00000000004004a1 <+34>:	mov    $0x1,%eax
>         0x00000000004004a6 <+39>:	leaveq 
>         0x00000000004004a7 <+40>:	retq   
>      
>      Without variable:
>         0x000000000040047f <+0>:	push   %rbp
>         0x0000000000400480 <+1>:	mov    %rsp,%rbp
>         0x0000000000400483 <+4>:	mov    $0x0,%eax
>         0x0000000000400488 <+9>:	callq  0x400474 <c>
>         0x000000000040048d <+14>:	cmp    $0x1,%eax
>         0x0000000000400490 <+17>:	jne    0x400499 <main+26>
>         0x0000000000400492 <+19>:	mov    $0x0,%eax
>         0x0000000000400497 <+24>:	jmp    0x40049e <main+31>
>         0x0000000000400499 <+26>:	mov    $0x1,%eax
>         0x000000000040049e <+31>:	leaveq 
>         0x000000000040049f <+32>:	retq   
>  
Yeah, definitely a worth optimization at the right place...


- Jan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-openlmi.rhcloud.com/r/874/#review1203
-----------------------------------------------------------


On Sept. 18, 2013, 2:28 p.m., Robin Hack wrote:
>  
>  -----------------------------------------------------------
>  This is an automatically generated e-mail. To reply, visit:
>  http://reviewboard-openlmi.rhcloud.com/r/874/
>  -----------------------------------------------------------
>  
>  (Updated Sept. 18, 2013, 2:28 p.m.)
>  
>  
>  Review request for OpenLMI Developers.
>  
>  
>  Repository: openlmi-providers
>  
>  
>  Description
>  -------
>  
>  Account: Better failure check of lock pool initialization.
>  
>  
>  Diffs
>  -----
>  
>    src/account/LMI_AccountProvider.c 5805f5e9637a77585c3eb63e75f8ae2bdfbbf063 
>  
>  Diff: http://reviewboard-openlmi.rhcloud.com/r/874/diff/
>  
>  
>  Testing
>  -------
>  
>  
>  Thanks,
>  
>  Robin Hack
>  
>  

_______________________________________________
openlmi-reviews mailing list
openlmi-reviews(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/openlmi-reviews

[Stephen Gallagher]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-openlmi.rhcloud.com/r/874/
-----------------------------------------------------------

(Updated Sept. 18, 2013, 12:51 p.m.)


Review request for OpenLMI Developers.


Repository: openlmi-providers


Description
-------

Account: Better failure check of lock pool initialization.


Diffs (updated)
-----

  src/account/LMI_AccountProvider.c 5805f5e9637a77585c3eb63e75f8ae2bdfbbf063 

Diff: http://reviewboard-openlmi.rhcloud.com/r/874/diff/


Testing
-------


Thanks,

Robin Hack


_______________________________________________
openlmi-reviews mailing list
openlmi-reviews(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/openlmi-reviews

[Stephen Gallagher]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-openlmi.rhcloud.com/r/874/#review1207
-----------------------------------------------------------

Ship it!


Ship It!

- Roman Rakus


On Sept. 18, 2013, 12:51 p.m., Robin Hack wrote:
>  
>  -----------------------------------------------------------
>  This is an automatically generated e-mail. To reply, visit:
>  http://reviewboard-openlmi.rhcloud.com/r/874/
>  -----------------------------------------------------------
>  
>  (Updated Sept. 18, 2013, 12:51 p.m.)
>  
>  
>  Review request for OpenLMI Developers.
>  
>  
>  Repository: openlmi-providers
>  
>  
>  Description
>  -------
>  
>  Account: Better failure check of lock pool initialization.
>  
>  
>  Diffs
>  -----
>  
>    src/account/LMI_AccountProvider.c 5805f5e9637a77585c3eb63e75f8ae2bdfbbf063 
>  
>  Diff: http://reviewboard-openlmi.rhcloud.com/r/874/diff/
>  
>  
>  Testing
>  -------
>  
>  
>  Thanks,
>  
>  Robin Hack
>  
>  

_______________________________________________
openlmi-reviews mailing list
openlmi-reviews(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/openlmi-reviews

[Stephen Gallagher]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-openlmi.rhcloud.com/r/874/#review1208
-----------------------------------------------------------

Ship it!


Ship It!

- Jan Synacek


On Sept. 18, 2013, 2:51 p.m., Robin Hack wrote:
>  
>  -----------------------------------------------------------
>  This is an automatically generated e-mail. To reply, visit:
>  http://reviewboard-openlmi.rhcloud.com/r/874/
>  -----------------------------------------------------------
>  
>  (Updated Sept. 18, 2013, 2:51 p.m.)
>  
>  
>  Review request for OpenLMI Developers.
>  
>  
>  Repository: openlmi-providers
>  
>  
>  Description
>  -------
>  
>  Account: Better failure check of lock pool initialization.
>  
>  
>  Diffs
>  -----
>  
>    src/account/LMI_AccountProvider.c 5805f5e9637a77585c3eb63e75f8ae2bdfbbf063 
>  
>  Diff: http://reviewboard-openlmi.rhcloud.com/r/874/diff/
>  
>  
>  Testing
>  -------
>  
>  
>  Thanks,
>  
>  Robin Hack
>  
>  

_______________________________________________
openlmi-reviews mailing list
openlmi-reviews(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/openlmi-reviews

[Stephen Gallagher]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard-openlmi.rhcloud.com/r/874/#review1209
-----------------------------------------------------------

Ship it!


Ship It!

- Michal Minar


On Sept. 18, 2013, 12:51 p.m., Robin Hack wrote:
>  
>  -----------------------------------------------------------
>  This is an automatically generated e-mail. To reply, visit:
>  http://reviewboard-openlmi.rhcloud.com/r/874/
>  -----------------------------------------------------------
>  
>  (Updated Sept. 18, 2013, 12:51 p.m.)
>  
>  
>  Review request for OpenLMI Developers.
>  
>  
>  Repository: openlmi-providers
>  
>  
>  Description
>  -------
>  
>  Account: Better failure check of lock pool initialization.
>  
>  
>  Diffs
>  -----
>  
>    src/account/LMI_AccountProvider.c 5805f5e9637a77585c3eb63e75f8ae2bdfbbf063 
>  
>  Diff: http://reviewboard-openlmi.rhcloud.com/r/874/diff/
>  
>  
>  Testing
>  -------
>  
>  
>  Thanks,
>  
>  Robin Hack
>  
>  

_______________________________________________
openlmi-reviews mailing list
openlmi-reviews(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/openlmi-reviews