[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Libcdio-devel] Rock Ridge deep directory support
From: |
Pete Batard |
Subject: |
Re: [Libcdio-devel] Rock Ridge deep directory support |
Date: |
Sat, 13 Jun 2020 13:36:01 +0100 |
User-agent: |
Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.9.0 |
Hi Thomas,
I'm happy to see you are doing better now. :)
On 2020.06.13 13:16, Thomas Schmitt wrote:
just to show that i began to review the Rock Ridge Deep Directory code,
Thanks!
i wonder why the RR-related code snippet in lib/iso9660/iso9660_fs.c
line 832 is not surrounded by #ifdef HAVE_ROCK and how a freshly calloc'ed
p_stat shall have (p_stat->rr.u_su_fields & ISO_ROCK_SUF_RE) true.
Further i wonder whether the ISO_ROCK_SUF_RE bit can only be set if
_iso9660_is_rock_ridge_enabled(p_image).
Line 819:
/* Reuse multiextent p_stat if not NULL */
if (!p_stat) {
p_stat = calloc(1, stat_len);
first_extent = true;
how could p_stat->rr.u_su_fields become non-zero ?
Technically we could have a non NULL last_p_stat and with p_stat =
last_p_stat the calloc is not guaranteed and p_stat->rr.u_su_fields
being zero is not guaranteed...
} else {
first_extent = false;
}
if (!p_stat) {
cdio_warn("Couldn't calloc(1, %d)", stat_len);
return NULL;
}
p_stat->type = (p_iso9660_dir->file_flags & ISO_DIRECTORY)
? _STAT_DIR : _STAT_FILE;
why no: #ifdef HAVE_ROCK ?
Because it's redundant. The ISO_ROCK_SUF_RE flag will be set to a non
zero only if HAVE_ROCK is enabled (because this is only done in
get_rock_ridge_filename() which is guarded), so there's no point in
adding the guard where it is not needed, and there is really no
performance hit by performing a simple test like this one always.
Line 832:
/* Ignore Rock Ridge Deep Directory RE entries */
why no: if (_iso9660_is_rock_ridge_enabled(p_image)) ?
Because this is performed in get_rock_ridge_filename() and
ISO_ROCK_SUF_RE can only be set if that is the case.
I explicitly designed the code around the idea that the flags we test
can only be set if:
- HAVE_ROCK is defined
- _iso9660_is_rock_ridge_enabled() is true
so that we don't have to re-check for the above when checking the flags.
Regards,
/Pete
if (p_stat->rr.u_su_fields & ISO_ROCK_SUF_RE)
goto fail;
why no: #endif ?
I will continue to read in a few hours.
Have a nice day :)
Thomas
- Re: [Libcdio-devel] Rock Ridge deep directory support, Thomas Schmitt, 2020/06/13
- Re: [Libcdio-devel] Rock Ridge deep directory support,
Pete Batard <=
- Re: [Libcdio-devel] Rock Ridge deep directory support, Thomas Schmitt, 2020/06/14
- Re: [Libcdio-devel] Rock Ridge deep directory support, Thomas Schmitt, 2020/06/14
- Re: [Libcdio-devel] Rock Ridge deep directory support, Pete Batard, 2020/06/16
- Re: [Libcdio-devel] Rock Ridge deep directory support, Rocky Bernstein, 2020/06/16
- Re: [Libcdio-devel] Rock Ridge deep directory support, Rocky Bernstein, 2020/06/16
- Re: [Libcdio-devel] Rock Ridge deep directory support, Pete Batard, 2020/06/16
- Re: [Libcdio-devel] Rock Ridge deep directory support, Rocky Bernstein, 2020/06/16
- Re: [Libcdio-devel] Rock Ridge deep directory support, Thomas Schmitt, 2020/06/16
- Re: [Libcdio-devel] Rock Ridge deep directory support, Pete Batard, 2020/06/16
- Re: [Libcdio-devel] Rock Ridge deep directory support, Rocky Bernstein, 2020/06/16