[U-Boot] [PATCH v2 18/23] efi_loader: implement a pseudo "file delete"

[Alexander Graf]

> Am 06.09.2018 um 04:43 schrieb AKASHI Takahiro <takahiro.akashi at linaro.org>:
> 
>> On Wed, Sep 05, 2018 at 10:22:07AM +0200, Alexander Graf wrote:
>> 
>> 
>>> On 05.09.18 04:51, AKASHI Takahiro wrote:
>>>> On Tue, Sep 04, 2018 at 11:16:38AM +0200, Alexander Graf wrote:
>>>> 
>>>> 
>>>>> On 04.09.18 09:49, AKASHI Takahiro wrote:
>>>>> This patch is necessary to run SCT.efi (UEFI Self-Certification Test).
>>>>> Returning EFI_SUCCESS can cheat SCT execution.
>>>>> 
>>>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
>>>> 
>>>> How hard would it be to implement a real delete instead?
>>> 
>>> Good question.
>>> The hardest part of implementation, I believe, is about handling
>>> nasty long file names in a directory, in particular, when
>>> directory entries which comprise a single LFN extend across two clusters.
>>> In this case, all the entries must be deleted first, and then go back
>>> to the first one and modify it. Due to a current simple buffering of
>>> cluster (only one cluster buffer a time), some tweak is necessary.
>>> In addition, if there is an already-deleted entry ahead of the given file,
>>> we need further rewind directory entries and update the "new" last valid
>>> one to mark it as so.
>>> Those kind of things would make the implementation a bit complicated
>>> rather than simple as expected.
>>> 
>>> So unless 'real' delete is a must for anything, I'm lukewarm to
>>> work on it.
>>> 
>>> # I admit that we don't have to have this patch just if *delete* returns
>>> successfully without doing anything.
>> 
>> I think both truncate-to-zero (this patch) and
>> report-success-when-failure (suggested patch) are hacks that shouldn't
>> really go upstream that way unfortunately.
>> 
>> The way I read the long file name definitions, I think it should be
>> perfectly ok to only remove the short file name directory entry.
> 
> Do you accept such an ugly implementation although it yet complies
> with fat specification?

It's valid for all intents and purposes, so why not :).

> 
>> Sure,
>> we're leaking a few directory entries for the LFN, but I don't think
>> that's too bad. Any fsck should be able to find those later on...
> 
> As I said in my cover-letter, fsck would complain any way.
> 
>> As for how to delete the entry without rewinding directory entries, you
>> can just set name[0]=DELETED_FLAG; to indicate that the SFN is deleted, no?
> 
> Yes if DELETED_FLAG=0x5e and if we ignore the specific case where
> name[0] be 0x00, which means the entry is the *first* invalid one.
> (again, the latter would not be mandatory in term of compatibility.)
> 
> Since I have already had the code, "real" delete will be put in my next
> version if nobody has concerns on this simple implementation.

Thanks, that would be great! :)

Alex