<div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div>Point regarding conclusive messages taken, will revert the relevant lines.</div><div dir="ltr">As for the time variable, all I did was to match it to the format as in the case of HAVE_UTIMENSAT, just above it.</div><div>Although the variable isn't used further, inlining it is unreadable.</div><div dir="ltr"><br></div><div>You also changed this:</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">ret = z_erofs_decompress(&(struct z_erofs_decompress_req) {</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="white-space:pre"> </span>.in = raw,</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="white-space:pre"> </span>.out = buffer,</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="white-space:pre"> </span>.decodedskip = 0,</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="white-space:pre"> </span>.inputsize = map.m_plen,</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="white-space:pre"> </span>.decodedlength = map.m_llen,</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="white-space:pre"> </span>.alg = map.m_algorithmformat,</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="white-space:pre"> </span>.partial_decoding = 0</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="white-space:pre"> </span> });</blockquote></blockquote><div> </div><div>to this:</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">struct z_erofs_decompress_req rq = {</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="white-space:pre"> </span>.in = raw,</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="white-space:pre"> </span>.out = buffer,</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="white-space:pre"> </span>.decodedskip = 0,</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="white-space:pre"> </span>.inputsize = map.m_plen,</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="white-space:pre"> </span>.decodedlength = map.m_llen,</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="white-space:pre"> </span>.alg = map.m_algorithmformat,</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="white-space:pre"> </span>.partial_decoding = 0</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">};</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">ret = z_erofs_decompress(&rq);</blockquote></blockquote><div><br></div><div>Although that variable could have remained avoided, like in lib/data.c, where it's still avoided:</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">ret = z_erofs_decompress(&(struct z_erofs_decompress_req) {</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="white-space:pre"> </span>.in = raw,</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="white-space:pre"> </span>.out = buffer + end - offset,</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="white-space:pre"> </span>.decodedskip = skip,</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="white-space:pre"> </span>.inputsize = map.m_plen,</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="white-space:pre"> </span>.decodedlength = length,</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="white-space:pre"> </span>.alg = map.m_algorithmformat,</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="white-space:pre"> </span>.partial_decoding = partial</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="white-space:pre"> </span> });</blockquote></blockquote><div><br></div><div>It's just that inconsistency in code style is an eyesore. ;) </div></div></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, 28 Jan 2022 at 15:22, Igor Ostapenko <<a href="mailto:igoreisberg@gmail.com">igoreisberg@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">From: Igor Eisberg <<a href="mailto:igoreisberg@gmail.com" target="_blank">igoreisberg@gmail.com</a>><br>
<br>
* Added useful error messages.<br>
* Most errors start with lower-case, let's make all of them lower-case<br>
for better consistency.<br>
<br>
Signed-off-by: Igor Ostapenko <<a href="mailto:igoreisberg@gmail.com" target="_blank">igoreisberg@gmail.com</a>><br>
---<br>
dump/main.c | 4 +++-<br>
fsck/main.c | 35 +++++++++++++++++++++++------------<br>
mkfs/main.c | 32 +++++++++++++++++---------------<br>
3 files changed, 43 insertions(+), 28 deletions(-)<br>
<br>
diff --git a/dump/main.c b/dump/main.c<br>
index 0616113..664780b 100644<br>
--- a/dump/main.c<br>
+++ b/dump/main.c<br>
@@ -162,8 +162,10 @@ static int erofsdump_parse_options_cfg(int argc, char **argv)<br>
}<br>
}<br>
<br>
- if (optind >= argc)<br>
+ if (optind >= argc) {<br>
+ erofs_err("missing argument: IMAGE");<br>
return -EINVAL;<br>
+ }<br>
<br>
cfg.c_img_path = strdup(argv[optind++]);<br>
if (!cfg.c_img_path)<br>
diff --git a/fsck/main.c b/fsck/main.c<br>
index 6f17d0e..5667f2a 100644<br>
--- a/fsck/main.c<br>
+++ b/fsck/main.c<br>
@@ -202,8 +202,10 @@ static int erofsfsck_parse_options_cfg(int argc, char **argv)<br>
}<br>
}<br>
<br>
- if (optind >= argc)<br>
+ if (optind >= argc) {<br>
+ erofs_err("missing argument: IMAGE");<br>
return -EINVAL;<br>
+ }<br>
<br>
cfg.c_img_path = strdup(argv[optind++]);<br>
if (!cfg.c_img_path)<br>
@@ -230,8 +232,12 @@ static void erofsfsck_set_attributes(struct erofs_inode *inode, char *path)<br>
<br>
if (utimensat(AT_FDCWD, path, times, AT_SYMLINK_NOFOLLOW) < 0)<br>
#else<br>
- if (utime(path, &((struct utimbuf){.actime = inode->i_ctime,<br>
- .modtime = inode->i_ctime})) < 0)<br>
+ const struct utimbuf time = {<br>
+ .actime = inode->i_ctime,<br>
+ .modtime = inode->i_ctime,<br>
+ };<br>
+<br>
+ if (utime(path, &time) < 0)<br>
#endif<br>
erofs_warn("failed to set times: %s", path);<br>
<br>
@@ -512,7 +518,7 @@ static inline int erofs_extract_dir(struct erofs_inode *inode)<br>
struct stat st;<br>
<br>
if (errno != EEXIST) {<br>
- erofs_err("failed to create directory %s: %s",<br>
+ erofs_err("failed to create directory: %s (%s)",<br>
fsckcfg.extract_path, strerror(errno));<br>
return -errno;<br>
}<br>
@@ -528,8 +534,11 @@ static inline int erofs_extract_dir(struct erofs_inode *inode)<br>
* Try to change permissions of existing directory so<br>
* that we can write to it<br>
*/<br>
- if (chmod(fsckcfg.extract_path, 0700) < 0)<br>
+ if (chmod(fsckcfg.extract_path, 0700) < 0) {<br>
+ erofs_err("failed to set permissions: %s (%s)",<br>
+ fsckcfg.extract_path, strerror(errno));<br>
return -errno;<br>
+ }<br>
}<br>
return 0;<br>
}<br>
@@ -551,18 +560,20 @@ again:<br>
erofs_warn("try to forcely remove directory %s",<br>
fsckcfg.extract_path);<br>
if (rmdir(fsckcfg.extract_path) < 0) {<br>
- erofs_err("failed to remove: %s",<br>
- fsckcfg.extract_path);<br>
+ erofs_err("failed to remove: %s (%s)",<br>
+ fsckcfg.extract_path, strerror(errno));<br>
return -EISDIR;<br>
}<br>
} else if (errno == EACCES &&<br>
chmod(fsckcfg.extract_path, 0700) < 0) {<br>
+ erofs_err("failed to set permissions: %s (%s)",<br>
+ fsckcfg.extract_path, strerror(errno));<br>
return -errno;<br>
}<br>
tryagain = false;<br>
goto again;<br>
}<br>
- erofs_err("failed to open %s: %s", fsckcfg.extract_path,<br>
+ erofs_err("failed to open: %s (%s)", fsckcfg.extract_path,<br>
strerror(errno));<br>
return -errno;<br>
}<br>
@@ -768,15 +779,15 @@ int main(int argc, char **argv)<br>
err = erofsfsck_check_inode(sbi.root_nid, sbi.root_nid);<br>
if (fsckcfg.corrupted) {<br>
if (!fsckcfg.extract_path)<br>
- erofs_err("Found some filesystem corruption");<br>
+ erofs_err("found some filesystem corruption");<br>
else<br>
- erofs_err("Failed to extract filesystem");<br>
+ erofs_err("failed to extract filesystem");<br>
err = -EFSCORRUPTED;<br>
} else if (!err) {<br>
if (!fsckcfg.extract_path)<br>
- erofs_info("No error found");<br>
+ erofs_info("no errors found");<br>
else<br>
- erofs_info("Extract data successfully");<br>
+ erofs_info("extracted filesystem successfully");<br>
<br>
if (fsckcfg.print_comp_ratio) {<br>
double comp_ratio =<br>
diff --git a/mkfs/main.c b/mkfs/main.c<br>
index c755da1..dd698ff 100644<br>
--- a/mkfs/main.c<br>
+++ b/mkfs/main.c<br>
@@ -381,9 +381,6 @@ static int mkfs_parse_options_cfg(int argc, char *argv[])<br>
}<br>
}<br>
<br>
- if (optind >= argc)<br>
- return -EINVAL;<br>
-<br>
if (cfg.c_blobdev_path && cfg.c_chunkbits < LOG_BLOCK_SIZE) {<br>
erofs_err("--blobdev must be used together with --chunksize");<br>
return -EINVAL;<br>
@@ -396,24 +393,29 @@ static int mkfs_parse_options_cfg(int argc, char *argv[])<br>
return -EINVAL;<br>
}<br>
<br>
+ if (optind >= argc) {<br>
+ erofs_err("missing argument: FILE");<br>
+ return -EINVAL;<br>
+ }<br>
+<br>
cfg.c_img_path = strdup(argv[optind++]);<br>
if (!cfg.c_img_path)<br>
return -ENOMEM;<br>
<br>
if (optind >= argc) {<br>
- erofs_err("Source directory is missing");<br>
+ erofs_err("missing argument: DIRECTORY");<br>
return -EINVAL;<br>
}<br>
<br>
cfg.c_src_path = realpath(argv[optind++], NULL);<br>
if (!cfg.c_src_path) {<br>
- erofs_err("Failed to parse source directory: %s",<br>
+ erofs_err("failed to parse source directory: %s",<br>
erofs_strerror(-errno));<br>
return -ENOENT;<br>
}<br>
<br>
if (optind < argc) {<br>
- erofs_err("Unexpected argument: %s\n", argv[optind]);<br>
+ erofs_err("unexpected argument: %s\n", argv[optind]);<br>
return -EINVAL;<br>
}<br>
if (quiet)<br>
@@ -456,7 +458,7 @@ int erofs_mkfs_update_super_block(struct erofs_buffer_head *bh,<br>
<br>
buf = calloc(sb_blksize, 1);<br>
if (!buf) {<br>
- erofs_err("Failed to allocate memory for sb: %s",<br>
+ erofs_err("failed to allocate memory for sb: %s",<br>
erofs_strerror(-errno));<br>
return -ENOMEM;<br>
}<br>
@@ -538,7 +540,7 @@ int parse_source_date_epoch(void)<br>
<br>
epoch = strtoull(source_date_epoch, &endptr, 10);<br>
if (epoch == -1ULL || *endptr != '\0') {<br>
- erofs_err("Environment variable $SOURCE_DATE_EPOCH %s is invalid",<br>
+ erofs_err("environment variable $SOURCE_DATE_EPOCH %s is invalid",<br>
source_date_epoch);<br>
return -EINVAL;<br>
}<br>
@@ -641,34 +643,34 @@ int main(int argc, char **argv)<br>
sb_bh = erofs_buffer_init();<br>
if (IS_ERR(sb_bh)) {<br>
err = PTR_ERR(sb_bh);<br>
- erofs_err("Failed to initialize buffers: %s",<br>
+ erofs_err("failed to initialize buffers: %s",<br>
erofs_strerror(err));<br>
goto exit;<br>
}<br>
err = erofs_bh_balloon(sb_bh, EROFS_SUPER_END);<br>
if (err < 0) {<br>
- erofs_err("Failed to balloon erofs_super_block: %s",<br>
+ erofs_err("failed to balloon erofs_super_block: %s",<br>
erofs_strerror(err));<br>
goto exit;<br>
}<br>
<br>
err = erofs_load_compress_hints();<br>
if (err) {<br>
- erofs_err("Failed to load compress hints %s",<br>
+ erofs_err("failed to load compress hints %s",<br>
cfg.c_compress_hints_file);<br>
goto exit;<br>
}<br>
<br>
err = z_erofs_compress_init(sb_bh);<br>
if (err) {<br>
- erofs_err("Failed to initialize compressor: %s",<br>
+ erofs_err("failed to initialize compressor: %s",<br>
erofs_strerror(err));<br>
goto exit;<br>
}<br>
<br>
err = erofs_generate_devtable();<br>
if (err) {<br>
- erofs_err("Failed to generate device table: %s",<br>
+ erofs_err("failed to generate device table: %s",<br>
erofs_strerror(err));<br>
goto exit;<br>
}<br>
@@ -681,7 +683,7 @@ int main(int argc, char **argv)<br>
<br>
err = erofs_build_shared_xattrs_from_path(cfg.c_src_path);<br>
if (err) {<br>
- erofs_err("Failed to build shared xattrs: %s",<br>
+ erofs_err("failed to build shared xattrs: %s",<br>
erofs_strerror(err));<br>
goto exit;<br>
}<br>
@@ -727,7 +729,7 @@ exit:<br>
erofs_exit_configure();<br>
<br>
if (err) {<br>
- erofs_err("\tCould not format the device : %s\n",<br>
+ erofs_err("\tcould not format the device : %s\n",<br>
erofs_strerror(err));<br>
return 1;<br>
}<br>
-- <br>
2.30.2<br>
<br>
</blockquote></div>