[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [PATCH 3/4] pidfile: stop making pidfile error a spec
From: |
Gonglei |
Subject: |
Re: [Qemu-trivial] [PATCH 3/4] pidfile: stop making pidfile error a special case |
Date: |
Fri, 31 Oct 2014 15:33:52 +0800 |
User-agent: |
Mozilla/5.0 (Windows NT 6.1; rv:11.0) Gecko/20120327 Thunderbird/11.0.1 |
On 2014/10/31 15:16, Michael Tokarev wrote:
> 31.10.2014 08:00, Gonglei wrote:
>> On 2014/10/30 23:07, Michael Tokarev wrote:
>>
>>> In case of -daemonize, we write non-zero to the daemon
>>> pipe only if pidfile creation failed, so the parent will
>>> report error about pidfile problem. There's no need to
>>> make special case for this, since all other errors are
>>> reported by the child just fine. Let the parent report
>>> error and simplify logic in os_daemonize().
>>>
>>> This way, we don't need os_pidfile_error() function, since
>>> it only prints error now, so put the error reporting printf
>>> into the only place where qemu_create_pidfile() is called,
>>> in vl.c.
>>>
>>> While at it, fix wrong identation in os_daemonize().
>>
>> s/identation/identification/
>
> No, the original word was the right one.
>
Sorry, I can't find 'identation' both dictionary and Google.
Your meaning 'indentation'?
>>> Signed-off-by: Michael Tokarev <address@hidden>
>>> ---
>>> include/qemu-common.h | 1 -
>>> os-posix.c | 29 ++++++-----------------------
>>> os-win32.c | 5 -----
>>> vl.c | 2 +-
>>> 4 files changed, 7 insertions(+), 30 deletions(-)
>>>
>>> diff --git a/include/qemu-common.h b/include/qemu-common.h
>>> index b87e9c2..f862214 100644
>>> --- a/include/qemu-common.h
>>> +++ b/include/qemu-common.h
>>> @@ -357,7 +357,6 @@ char *qemu_find_file(int type, const char *name);
>>> void os_setup_early_signal_handling(void);
>>> char *os_find_datadir(void);
>>> void os_parse_cmd_args(int index, const char *optarg);
>>> -void os_pidfile_error(void);
>>>
>>> /* Convert a byte between binary and BCD. */
>>> static inline uint8_t to_bcd(uint8_t val)
>>> diff --git a/os-posix.c b/os-posix.c
>>> index eada8d4..a3b96d9 100644
>>> --- a/os-posix.c
>>> +++ b/os-posix.c
>>> @@ -221,18 +221,12 @@ void os_daemonize(void)
>>> do {
>>> len = read(fds[0], &status, 1);
>>> } while (len < 0 && errno == EINTR);
>>> - if (len != 1) {
>>> - exit(1);
>>> - }
>>
>> Does this check need to be removed?
>
> Yes, because...
>>
>>> - else if (status == 1) {
>>> - fprintf(stderr, "Could not acquire pidfile\n");
>>> - exit(1);
>>> - } else {
>>> - exit(0);
>>> - }
>>> - } else if (pid < 0) {
>>> - exit(1);
>>> - }
>>> +
>>> + exit(len == 1 && status == 0 ? 0 : 1);
>
> ...it is checked here, note the 'len == 1' part of the condition.
>
If len != 1, the original code exit with 1, after your changes,
it will exit with 0. Right?
> And here comes the wrong original identation -- this part of "else"
> was indented once too many:
>
I know your meaning now. :)
>>> +
>>> + } else if (pid < 0) {
>>> + exit(1);
>>> + }
>>>
>
> Thanks,
>
> /mjt
>
- [Qemu-trivial] [PATCH 0/4] cleanup -daemonize and pidfile creation a bit, Michael Tokarev, 2014/10/31
- [Qemu-trivial] [PATCH 3/4] pidfile: stop making pidfile error a special case, Michael Tokarev, 2014/10/31
- Re: [Qemu-trivial] [PATCH 3/4] pidfile: stop making pidfile error a special case, Gonglei, 2014/10/31
- Re: [Qemu-trivial] [PATCH 3/4] pidfile: stop making pidfile error a special case, Michael Tokarev, 2014/10/31
- Re: [Qemu-trivial] [PATCH 3/4] pidfile: stop making pidfile error a special case,
Gonglei <=
- Re: [Qemu-trivial] [PATCH 3/4] pidfile: stop making pidfile error a special case, Michael Tokarev, 2014/10/31
- Re: [Qemu-trivial] [PATCH 3/4] pidfile: stop making pidfile error a special case, Gonglei, 2014/10/31
- Re: [Qemu-trivial] [PATCH 3/4] pidfile: stop making pidfile error a special case, Michael Tokarev, 2014/10/31
- Re: [Qemu-trivial] [PATCH 3/4] pidfile: stop making pidfile error a special case, Gonglei, 2014/10/31
[Qemu-trivial] [PATCH 1/4] os-posix: use global daemon_pipe instead of cryptic fds[1], Michael Tokarev, 2014/10/31
[Qemu-trivial] [PATCH 4/4] os-posix: reorder parent notification for -daemonize, Michael Tokarev, 2014/10/31
[Qemu-trivial] [PATCH 2/4] os-posix: replace goto again with a proper loop, Michael Tokarev, 2014/10/31