On (01/02/16 14:33), Lukas Slebodnik wrote:
On (01/02/16 09:31), Lukas Slebodnik wrote:
>On (30/01/16 00:00), Jakub Hrozek wrote:
>>On Fri, Jan 29, 2016 at 08:24:00PM +0100, Lukas Slebodnik wrote:
>>> On (29/01/16 18:50), Jakub Hrozek wrote:
>>> >On Fri, Jan 29, 2016 at 03:25:56PM +0100, Lukas Slebodnik wrote:
>>> >> On (29/01/16 13:51), Pavel Reichl wrote:
>>> >> >
>>> >> >
>>> >> >On 01/29/2016 01:41 PM, Lukas Slebodnik wrote:
>>> >> >>https://fedorahosted.org/sssd/ticket/2931
>>> >> >>---
>>> >> >> src/providers/krb5/krb5_child.c | 17 +++++++++++++++++
>>> >> >> 1 file changed, 17 insertions(+)
>>> >> >>
>>> >> >>diff --git a/src/providers/krb5/krb5_child.c
b/src/providers/krb5/krb5_child.c
>>> >> >>index
12eb9e2093d2bdd7d67e8d029fec1455488aa67c..88bcaddc419c1e6dc5d9a0c69b50c45a45c95efc 100644
>>> >> >>--- a/src/providers/krb5/krb5_child.c
>>> >> >>+++ b/src/providers/krb5/krb5_child.c
>>> >> >>@@ -2675,6 +2675,23 @@ int main(int argc, const char
*argv[])
>>> >> >> goto done;
>>> >> >> }
>>> >> >>
>>> >> >>+ ret = open("/etc/krb5.conf", O_RDONLY);
>>> >> >>+ if (ret == EOK)
>>> >> >
>>> >> >I thought that open() returns file descriptor on success and and
-1 in case of error. Was I wrong?
>>> >> >
>>> >> That's a fast codding style change swith->if after testing
and before sending
>>> >> patch.
>>> >
>>> >It's better to test file access with open if you need to actually
use
>>> >the file to avoid TOCTOU-style races, but here I guess it doesn't
>>> >matter. Nonetheless, I would prefer to use a variable named 'fd'
and not
>>> >'ret'
>>> I was just lazy to declare another variable
>>
>>If you don't mind, I would prefer fd. When I quickly scan code, 'ret'
is
>>something we normally use for return codes. Here it's something that
>>actually represents a resource that needs to be closed later.
>>
>>>
>>> >and close the fd right away,
>>> It's already in patch.
>>
>>You're right, sorry, I overlooked that.
>Updated patch is attached.
>
>LS
I accidentally sent wrong patch.
Updated patch us variable fd for open and
code was moved to separate function.
Updated version is attached.
LS