[Freeipa-devel] [PATCHES] 206-209 Add default CFLAGS & fix hardened build

[Jan Cholasta]

On 6.12.2013 12:57, Alexander Bokovoy wrote:
> On Fri, 06 Dec 2013, Petr Viktorin wrote:
>> On 12/06/2013 11:52 AM, Jan Cholasta wrote:
>>> On 5.12.2013 13:31, Alexander Bokovoy wrote:
>>>> On Thu, 05 Dec 2013, Petr Viktorin wrote:
>>>>> On 12/05/2013 11:15 AM, Jan Cholasta wrote:
>>>>>> Hi,
>>>>>>
>>>>>> the attached patches fix
>>>>>> <https://fedorahosted.org/freeipa/ticket/3896>.
>>>>>>
>>>>>> Patch 207 should fix build failures some of you were having after
>>>>>> hardenening was enabled in the spec file.
>>>>>
>>>>> Thanks!
>>>>>
>>>>> In 209, would (ret != 1) make more sense than (ret == -1)? AFAIU zero
>>>>> would also indicate a problem, if it somehow ended up being returned.
>>>> no, write() returns -1 if an error has happened and amount of the data
>>>> written otherwise. We specifically need to check that there was an
>>>> error, not that we wrote more or less than were supposed to write.
>>>>
>>>> We are looking for EPIPE and EINTR mostly, since other errors make
>>>> little
>>>> difference for our case. In case of EINTR we need to repeat or the
>>>> worker thread didn't receive our shutdown request. In case of EPIPE we
>>>> will also get SIGPIPE and in general this means the other thread died..
>>>>
>>>> However, even if writing to the pipe failed, we still need to wait
>>>> until
>>>> thread dies with pthread_join(). I think returning -1 here is
>>>> premature.
>>>
>>> Fixed, updated patches attached.
>>>
>>> Also removed CFLAGS duplication, see patch 212.
>>
>> Thanks you! The build works fine, ACK for 206-208, 212.
>>
>> Alexander, is the C part OK? It seems a do-while loop would make sense
>> here.
> I think do-while would be cleaner, purely from intent expression point
> of view.
>

I actually like it the way it is, because it follows the

    ret = func()
    if (ret == -1) {
    }

pattern, which is used abundantly in our code.

-- 
Jan Cholasta