Rationale as explained in#1158 (comment) …

Hi@liconghui628, thanks for creating an issue + PR.

@geky AFAICT, the place where the tag is checked we expect valid data, which is not the case. Given the call inlfs_dir_fetch provides placeholder values for theftag might be treated as a sign to ignore the callback. A more robust check would amend

Hmmm. It's truelfs_dir_fetch/lfs_dir_fetchmatch need to written defensively and expect unwritten/corrupt storage. But I don't think this can be reached, even if corrupt?

Earlier in this loop we check if the tag is "valid" (if the sign bit is clear), andbreak if it's not:

lfs_tag_isvalid impl:

Nothing else touchestag between that check and(fmask & tag) == (fmask & ftag), so, unless I'm missing something, the callback should never be called whenfmask=ftag=-1.

Short of in-RAM corruption or some other bug.

Hi@liconghui628, thanks for creating an issue + PR.

@geky AFAICT, the place where the tag is checked we expect valid data, which is not the case. Given the call inlfs_dir_fetch provides placeholder values for theftag might be treated as a sign to ignore the callback. A more robust check would amend

Hmmm. It's truelfs_dir_fetch/lfs_dir_fetchmatch need to written defensively and expect unwritten/corrupt storage. But I don't think this can be reached, even if corrupt?

Earlier in this loop we check if the tag is "valid" (if the sign bit is clear), andbreak if it's not:

littlefs/lfs.c

Lines 1182 to 1186 inadad0fb

	// next commit not yet programmed?
	if (!lfs_tag_isvalid(tag)) {
	// we only might be erased if the last tag was a crc
	maybeerased= (lfs_tag_type2(ptag)==LFS_TYPE_CCRC);
	break;

lfs_tag_isvalid impl:

littlefs/lfs.c

Lines 351 to 353 inadad0fb

	staticinlineboollfs_tag_isvalid(lfs_tag_ttag) {
	return !(tag&0x80000000);
	}

Nothing else touchestag between that check and(fmask & tag) == (fmask & ftag), so, unless I'm missing something, the callback should never be called whenfmask=ftag=-1.

Short of in-RAM corruption or some other bug.

@geky This question is indeed very strange. I tried to add some printing information, and in fact, it did call up here.

[2025-10-20 18:59:54]  lfs_trace:3730: lfs_mount -> 0[2025-10-20 18:59:54]  milfs_mount_log 282 lfs_mout ret 0[2025-10-20 18:59:54]  lfs_trace:1962: lfs_dir_open(62FD54B8, 62FCE2C4, "/")[2025-10-20 18:59:54]  lfs_trace:1974: lfs_dir_open -> 0x3ff[2025-10-20 18:59:54]  lfs_dir_fetchmatch 767[2025-10-20 18:59:54]  milfs_flash_read_log block=1 off=0[2025-10-20 18:59:54]  lfs_dir_fetchmatch 783[2025-10-20 18:59:54]  lfs_dir_fetchmatch 791[2025-10-20 18:59:54]  lfs_dir_fetchmatch 874[2025-10-20 18:59:54]  lfs_dir_fetchmatch 891[2025-10-20 18:59:54]  lfs_dir_fetchmatch 895[2025-10-20 18:59:54]  lfs_dir_fetchmatch 929 cb=00000000[2025-10-20 18:59:54]  Exception Entry--->>>[2025-10-20 18:59:54]  mcause 38000001, mepc 00000000, mtval 00000000[2025-10-20 18:59:54]  Exception code: 1[2025-10-20 18:59:54]    msg: Instruction access fault[2025-10-20 18:59:54]  === backtrace ===[2025-10-20 18:59:54]  addr2line -e <your elf file>.elf -fp 0xa00a715a 0xa0027dbc 0x0 0xa013cd30 0xa013fbd4 0xa013feac 0xa006db38 [2025-10-20 18:59:54]  [2025-10-20 18:59:54]  -+-+-+- BFLB COREDUMP v0.0.1 +-+-+-+

lfs.c

Ah!@liconghui628, this looks like an older version of the codebase.

In particular it's missing this patch:#337, which was merged inv2.1.4.

I believe this is the same issue.

Would it make sense to include the NULL pointer check anyway; and be it as an assertion at the call site ofcb? That way you could at least provide a somewhat reasonable message for debugging in case there was some regression.

Would it make sense to include the NULL pointer check anyway; and be it as an assertion at the call site ofcb? That way you could at least provide a somewhat reasonable message for debugging in case there was some regression.

I'm not opposed to it, but what would it really add over hardfaulting? Both point out thiscb is NULL when it shouldn't be.

There's a lot of things that can be NULL in C, fortunately even microcontrollers these days make NULL dereferences "safe" and easy to find. The log@liconghui628 shared even had a full backtrace +addr2line invocation we could have looked at if we needed more info.

@geky@BenBE Thank you for your patient explanation. From the perspective of internal implementation alone, I think NULL detection is necessary because it is uncertain whether external modifications will be executed here in the future, the impact on business is quite significant. We just need to make the interface implementation more robust, without worrying about how it is called externally.

Would it make sense to include the NULL pointer check anyway; and be it as an assertion at the call site ofcb?

I'm not opposed to it, but what would it really add over hardfaulting? Both point out this cb is NULL when it shouldn't be.

IMO, it's valuable to make intent/expectations explicit betweenassert(ptr != NULL) (ptr should never be NULL) andif (ptr == NULL) return; (it might beNULL, just ignore it). It can help in reasoning/debugging, even if the assert behavior were no different than letting it hard fault.

I'm still not convinced an assert adds value over just dereferencing the callback, but we can add anLFS3_ASSERT(cb != NULL) for the self-documenting reason, and because this logic is so complicated.

@liconghui628, if you change this toLFS3_ASSERT(cb != NULL), you can still get a runtime error by defining LFS3_ASSERT to return. See#514.

This is currently tested in CI and won't change without a new major version.