On Thu, May 21, 2020 at 3:10 AM Arvind Raghavan
<raghavan.arvind@xxxxxxxxx> wrote:
>
> Allow regular links and symlinks to be passed as input to fssum.
>
> Signed-off-by: Arvind Raghavan <raghavan.arvind@xxxxxxxxx>
> Signed-off-by: Jayashree Mohan <jaya@xxxxxxxxxxxxx>
> Signed-off-by: Vijay Chidambaram <vijay@xxxxxxxxxxxxx>
> ---
> src/fssum.c | 35 ++++++++++++++++++++++++++++++++++-
> 1 file changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/src/fssum.c b/src/fssum.c
> index ece0f556..2d1624ca 100644
> --- a/src/fssum.c
> +++ b/src/fssum.c
> @@ -29,6 +29,7 @@
> #include <inttypes.h>
> #include <assert.h>
> #include <endian.h>
> +#include <libgen.h>
>
> #define CS_SIZE 16
> #define CHUNKS 128
> @@ -884,8 +885,40 @@ main(int argc, char *argv[])
> if (gen_manifest)
> fprintf(out_fp, "Flags: %s\n", flagstring);
>
> + struct stat64 path_st;
> + if (fstat64(fd, &path_st)) {
> + perror("fstat");
> + exit(-1);
> + }
> +
> sum_init(&cs);
> - sum(fd, 1, &cs, path, "");
> +
> + if (S_ISDIR(path_st.st_mode)) {
> + sum(fd, 1, &cs, path, "");
> + } else if (S_ISREG(path_st.st_mode) || S_ISLNK(path_st.st_mode)) {
> + // Copy because dirname may modify path
> + char* path_copy = alloc(strlen(path));
> + strcpy(path_copy, path);
> +
> + char* dir_path = dirname(path);
> + char* name = basename(path_copy);
> +
> + int dirfd = open(dir_path, O_RDONLY);
> + if (fd == -1) {
> + fprintf(stderr, "failed to open %s: %s\n", dir_path,
> + strerror(errno));
> + exit(-1);
> + }
> +
> + sum_one(dirfd, 1, &cs, dir_path, "", name);
Instead of all of the above, how about just:
sum_one(fd, 1, &cs, path, "", "");
>From looking at sum_one() code, it seems to me like that will work,
but I may be missing something.
It's not that you *want* the name in the checksum, it is not even
part of the metadata that is being synced with fsync.
Other than that patch set looks excellent.
Very pleasant for review :-)
One little thing is missing from the cover letter -
Which tests did you run to verify these changes do not regress existing
tests?
Feel free to add to all the other patches in the series:
Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>
If you are able to change sum_one() of empty name as I suggested,
feel free to add that to this patch as well.
Thanks,
Amir.
