[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]