[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v9 09/21] jobs: use job locks also in the unit tests
From: |
Emanuele Giuseppe Esposito |
Subject: |
Re: [PATCH v9 09/21] jobs: use job locks also in the unit tests |
Date: |
Wed, 20 Jul 2022 16:22:52 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0 |
Am 20/07/2022 um 15:06 schrieb Vladimir Sementsov-Ogievskiy:
> On 7/19/22 15:00, Emanuele Giuseppe Esposito wrote:
>>
>>
>> Am 11/07/2022 um 15:08 schrieb Vladimir Sementsov-Ogievskiy:
>>>
>>> That made me ask:
>>>
>>> 1. Are all tests always run in main loop? If yes, why to protect status
>>> reading in test_complete_in_standby() ?
>>>
>>> 2. Maybe, we don't need to protect anything here? Why to protect other
>>> things if we run everything in main loop?
>>
>> I think it's still good example and practice to protect a function if it
>> needs to be protected and its name ends with _locked. It would just
>> confuse the reader if we don't protect it.
>>
>
> Agree. But still, I think we should be consistent in such decisions. If
> you don't want to protect job->status in tests, then you shouldn't
> protect it in test_complete_in_standby() as well, just to not confuse
> someone who read the code.
>
>
Ok, I will protect job->status in those tests too.
Emanuele
- [PATCH v9 06/21] job: move and update comments from blockjob.c, (continued)
[PATCH v9 20/21] blockjob: remove unused functions, Emanuele Giuseppe Esposito, 2022/07/06
[PATCH v9 10/21] block/mirror.c: use of job helpers in drivers to avoid TOC/TOU, Emanuele Giuseppe Esposito, 2022/07/06
[PATCH v9 15/21] job.c: enable job lock/unlock and remove Aiocontext locks, Emanuele Giuseppe Esposito, 2022/07/06
[PATCH v9 21/21] job: remove unused functions, Emanuele Giuseppe Esposito, 2022/07/06
[PATCH v9 18/21] blockjob: rename notifier callbacks as _locked, Emanuele Giuseppe Esposito, 2022/07/06
Re: [PATCH v9 00/21] job: replace AioContext lock with job_mutex, Vladimir Sementsov-Ogievskiy, 2022/07/11