Re: [PATCH v27 07/10] fs/ntfs3: Add NTFS journal

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

below are a few comments based on a cppcheck run.
Don't take it too seriously into consideration. It could be either some useless code that could be removed or some issues in the logic.

It is reported only if a new iteration is done and if it makes sense to change something.

CJ

Le 29/07/2021 à 15:49, Konstantin Komarov a écrit :
This adds NTFS journal

Signed-off-by: Konstantin Komarov <almaz.alexandrovich@xxxxxxxxxxxxxxxxxxxx>
---
  fs/ntfs3/fslog.c | 5181 ++++++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 5181 insertions(+)
  create mode 100644 fs/ntfs3/fslog.c

diff --git a/fs/ntfs3/fslog.c b/fs/ntfs3/fslog.c
new file mode 100644
index 000000000..53da12252
--- /dev/null
+++ b/fs/ntfs3/fslog.c

[...]

+/*
+ * last_log_lsn
+ *
+ * This routine walks through the log pages for a file, searching for the
+ * last log page written to the file
+ */
+static int last_log_lsn(struct ntfs_log *log)
+{

[...]

+tail_read:
+	first_tail = first_tail_prev;
+	final_off = final_off_prev;
+
+	second_tail = second_tail_prev;
+	second_off = second_off_prev;
+
+	page_cnt = page_pos = 1;
+
+	curpage_off = seq_base == log->seq_num ? min(log->next_page, page_off)
+					       : log->next_page;
+
+	wrapped_file =
+		curpage_off == log->first_page &&
+		!(log->l_flags & (NTFSLOG_NO_LAST_LSN | NTFSLOG_REUSE_TAIL));
+
+	expected_seq = wrapped_file ? (log->seq_num + 1) : log->seq_num;
+
+	nextpage_off = curpage_off;

This 'nextpage_off' is overwritten a few lines below.
Either it is useless, either there is an issue in the logic.

+
+next_page:
+	tail_page = NULL;
+	/* Read the next log page */
+	err = read_log_page(log, curpage_off, &page, &usa_error);
+
+	/* Compute the next log page offset the file */
+	nextpage_off = next_page_off(log, curpage_off);
+	wrapped = nextpage_off == log->first_page;
here.

+
+	if (tails > 1) {
+		struct RECORD_PAGE_HDR *cur_page =
+			Add2Ptr(page_bufs, curpage_off - page_off);
+
+		if (curpage_off == saved_off) {
+			tail_page = cur_page;
+			goto use_tail_page;
+		}
+

[...]

+int log_replay(struct ntfs_inode *ni, bool *initialized)
+{

[...]

+
+	vcn = le64_to_cpu(lrh->target_vcn);
+	vcn &= ~(log->clst_per_page - 1);
+

This 'vcn' is overwritten a few lines below.
Either it is useless, either there is an issue in the logic.

+add_allocated_vcns:
+	for (i = 0, vcn = le64_to_cpu(lrh->target_vcn),

here

+	    size = (vcn + 1) << sbi->cluster_bits;
+	     i < t16; i++, vcn += 1, size += sbi->cluster_size) {
+		attr = oa->attr;
+		if (!attr->non_res) {
+			if (size > le32_to_cpu(attr->res.data_size))
+				attr->res.data_size = cpu_to_le32(size);
+		} else {
+			if (size > le64_to_cpu(attr->nres.data_size))
+				attr->nres.valid_size = attr->nres.data_size =
+					attr->nres.alloc_size =
+						cpu_to_le64(size);
+		}
+	}
+
+	t16 = le16_to_cpu(lrh->undo_op);
+	if (can_skip_action(t16))
+		goto read_next_log_undo_action;
+
+	/* Point to the Redo data and get its length */
+	data = Add2Ptr(lrh, le16_to_cpu(lrh->undo_off));
+	dlen = le16_to_cpu(lrh->undo_len);
+
+	/* it is time to apply the undo action */
+	err = do_action(log, oe, lrh, t16, data, dlen, rec_len, NULL);

This 'err' is unused.
Maybe there is nothing that we can do, or the error handling is missing.

+
+read_next_log_undo_action:
+	/*
+	 * Keep reading and looping back until we have read the
+	 * last record for this transaction
+	 */
+	err = read_next_log_rec(log, lcb, &rec_lsn);
+	if (err)
+		goto out;
+

[...]



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux