Thank you for continuing the discussion.
The reply was delayed to summarize the arguing points.
> I already gave my comment on previous thread, and I prefer de array handling
> I sent instead of only two entries.
We haven't discussed enough yet and I have some questions.
I still don't understand what's technically problem.
> It will avoid repetitive loops to get entries from cache buffer.
Is that loop the first verification and name extraction?
I don't understand why you can avoid the repetitive loop by using arrays.
I think getting from an array is equivalent to getting it via a function.
A = obj->array[idx];
B = get(obj, idx);
For using, what's the difference between A and B?
> I think it is also suitable for function definition and union type entry
> structure.
I, too, think the combination is not bad.
However, it has no advantage compared to other methods.
(Can you give me any example?)
Also, as I said in my previous mail, union has the problem of too flexible for
type.
(Especially file-de and stream-de are easy to confuse)
So I want to avoid to access union directly from the upper function, as
possible.
> If you send the patches included this change again, I will actively look into
> your patches.
It will take some time as I haven't come up with a good idea yet.
We have discussed it so far, but there are still some unclear points.
First, I would like to clarify them.
1. About the need for TYPE_NAME-validation in exfat_get_dentry_set().
My opinion is
> It is possible to correctly determine that
> "Immediately follow the Stream Extension directory entry as a consecutive
> series"
> whether the TYPE_NAME check is implemented exfat_get_uniname_from_ext_entry()
> or
> exfat_get_dentry_set().
> It's functionally same, so it is also right to validate in either.
Your opinion is
> We have not checked the problem when it is removed because it was implemented
> according to the specification from the beginning.
I understand that you haven't thought about it yet.
What happens if I don't check here?
Please imagine if you can.
2. About TYPE_NAME-validation in exfat_get_uniname_from_ext_entry()
Below are the changes in '[PATCH v4 1/5] exfat: integrates dir-entry getting
and validation'
> - for (i = 2; i < es->num_entries; i++) {
> - struct exfat_dentry *ep = exfat_get_dentry_cached(es, i);
> -
> - /* end of name entry */
> - if (exfat_get_entry_type(ep) != TYPE_EXTEND)
> - break;
>
> + i = ES_INDEX_NAME;
> + while ((ep = exfat_get_validated_dentry(es, i++, TYPE_NAME))) {
> exfat_extract_uni_name(ep, uniname);
> uniname += EXFAT_FILE_NAME_LEN;
> }
Your request for this change is
> Please find the way to access name entries like ep_file, ep_stream
> without calling exfat_get_validated_dentry().
What is the reason(or rationale) for such a request?
Please explain what the problem is with this change, if you can.
As I explained before, the reason for validating TYPE_NAME here is
> name-length and type validation and name-extraction should not be separated.
> These are closely related, so these should be placed physically and
> temporally close.
Please explain why you shouldn't validate TYPE_NAME here.
3. About using exfat_get_validated_dentry() in
exfat_update_dir_chksum_with_entry_set()
Below are the changes in '[PATCH v4 1/5] exfat: integrates dir-entry getting
and validation'
> for (i = 0; i < es->num_entries; i++) {
> - ep = exfat_get_dentry_cached(es, i);
> + ep = exfat_get_validated_dentry(es, i, TYPE_ALL);
> chksum = exfat_calc_chksum16(ep, DENTRY_SIZE, chksum,
> chksum_type);
Your request for this change is
> Ditto, You do not need to repeatedly call exfat_get_validated_dentry() for
> the entries
> which got from exfat_get_dentry_set().
Even if the entry was got from exfat_get_dentry_set(), we need to get the ep
again to calculate the checksum.
exfat_get_validated_dentry() with TYPE_ALL is the same as
exfat_get_dentry_cached() because it allows all TYPEs.
Please elaborate on what the problem is.
4. About double-checking name entries as TYPE_SECONDARY and TYPE_NAME.
You said in 'RE: [PATCH v3] exfat: integrates dir-entry getting and validation'.
> your v3 patch are
> already checking the name entries as TYPE_SECONDARY. And it check them with
> TYPE_NAME again in exfat_get_uniname_from_ext_entry(). If you check TYPE_NAME
> with stream->name_len, We don't need to perform the loop for extracting
> filename from the name entries if stream->name_len or name entry is invalid.
It is rare case that stream->name_len or name-entry are invalid.
Perform the loop to extract filename when stream->name_len or name-entry is
invalid has little effect.
What is the probrem with perform the loop for extract filename when
stream->name_len or name-entry are invalid?
5. About validate flags as argument of exfat_get_dentry_set().
You suggested
> You can add a validate flags as argument of exfat_get_dentry_set(), e.g.
> none, basic and strict.
> So as I suggested earlier, You can make it with an argument flags so that we
> skip the validation.
What are the advantages of skipping validation with this flag?
I don't think there's any advantage worth the complexity of the code.
This discussion may take some time, but I hope you continue the discussion.
BR
---
Tetsuhiro Kohada <[email protected]>
