10.07.2023 09:27, Murphy Zhou пишет:
Hi,
Sorry for the late reply.
On Thu, Jul 6, 2023 at 4:54 PM stsp <stsp2@xxxxxxxxx> wrote:
What while loop do you mean and what
doesn't hurt? Does the rest of the patch
look ok?
I mean the do-while loop ensures a new semaphore is created.
But it would really help if you quote
the relevant part of the patch, as I
have no idea what loop ensures a sem
is created. If I look through the patch
to find any loop that it removes, then
its this one:
- do {
- memset(&sem_ds, 0, sizeof(sem_ds));
- semu.buf = &sem_ds;
- ret = semctl(semid, 0, IPC_STAT, semu);
- } while (!(ret == 0 && sem_ds.sem_otime != 0));
Does it ensure if sem is created - no,
it only waits until otime is non-zero.
AFAIK the formal review process asks
you to comment with the relevant
quoting, so may I ask you to please
do the same. :)
And in most of the test scenarios, this program runs only once and
there is no semaphore left behind, unless debugging this program
itself.
Indeed, so the problem was noticed by
me when debugging, rather than by
someone else who only runs it.
So I don't think there are any race conditions being opened up.
I don't think it is a good idea to have a
race conditions for those who can press
^c during tests, even if otherwise the
problem is unobservable.
With my patch the recovery after ^c
works reliably.
But much more importantly the
"Wait initialization complete."
loop didn't work, and now it does.
That is a primary reason of the patch.
The sem rm fix is just for debugging.
