subsys: fs: bm_zms: remove global callbacks array

[MirkoCovizzi]

Removes global callbacks array and replaces with callback
member inside struct bm_zms_fs.
This fixes a bug where registering multiple callbacks on
the same struct bm_zms_fs would update the user reference
number incrementally and saturate the global callbacks array.
When calling bm_zms_clear, this would only reset to NULL
the last callback set, leaving the rest still set.
This update makes it so that there can only be one callback
per each struct bm_zms_fs instance, mirroring the behavior
of other libraries.
Due to this change, also removes the BM_ZMS_MAX_USERS
Kconfig option.

[github-actions]

You can find the documentation preview for this PR here.

[rghaddab]

@MirkoCovizzi I do not understand this bug.
The current bm_zms behavior should be that only one callback is registered for a bm_zms_fs structure.
The caller will get its user_num which indicates its order in the callback register.
No other bm_zms_register should be called on the same structure before clearing the previous one.
Can you detail the usecases where this could happen ?

[lemrey]

I suppose that's one way to fix it. It does however add one limitation, which is that only the owner of an instance can receive callbacks from it, thus forcing a single owner for each instance. In practice, it is no longer possible to share a partition, every instance would need its own.

But it may be fine, first, because there a no events that are relevant for the whole application (like GC was). And second, because the application could still be designed in a way that allows to share a partition, by wrapping NVM operations.

[MirkoCovizzi]

@MirkoCovizzi I do not understand this bug. The current bm_zms behavior should be that only one callback is registered for a bm_zms_fs structure. The caller will get its user_num which indicates its order in the callback register. No other bm_zms_register should be called on the same structure before clearing the previous one. Can you detail the usecases where this could happen ?

It is not a use case. The current implementation makes it possible for a user to call the register function on the same instance multiple times, each time incrementing the user_num. When clearing, only the last registration is reset. If this behavior is not expected and should not be happening, then it's a bug.
This can be solved in multiple ways, however, since as you also say the user is expected to call register on an instance at most once, then it stands to reason that there can be a member inside the structure that holds the function pointer, the same way we do in the storage subsystem.

[MirkoCovizzi]

I suppose that's one way to fix it. It does however add one limitation, which is that only the owner of an instance can receive callbacks from it, thus forcing a single owner for each instance. In practice, it is no longer possible to share a partition, every instance would need its own.

But it may be fine, first, because there a no events that are relevant for the whole application (like GC was). And second, because the application could still be designed in a way that allows to share a partition, by wrapping NVM operations.

What are some examples of situations where an application might want to share a single partition between different instances of a file system (or storage)?

[anhmolt]

I don't see the cb_registered flag used for anything. Remove it?

[eivindj-nordic]

Suggested change

	* @param p_evt The event.
	* @param evt The event.

[MirkoCovizzi]

This will be addressed separately

[eivindj-nordic]

Suggested change

	typedef void (*bm_zms_cb_t)(bm_zms_evt_t const *p_evt);
	typedef void (*bm_zms_cb_t)(bm_zms_evt_t const *evt);

[peknis]

Suggested change

	* Removed the :kconfig:option:`CONFIG_BM_ZMS_MAX_USERS` Kconfig option. Now the library expects at most one callback for each instance of the struct :c:struct:`bm_zms_fs`.
	* Removed the :c:member:`bm_zms_init_flags.cb_registred` member since it was not used anymore.
	* Removed:
	* The :kconfig:option:`CONFIG_BM_ZMS_MAX_USERS` Kconfig option.
	Now, the library expects at most one callback for each instance of the :c:struct:`bm_zms_fs` structure.
	* The :c:member:`bm_zms_init_flags.cb_registred` member as it was not used anymore.

[peknis]

Suggested change

	* The :kconfig:option:`CONFIG_BM_ZMS_MAX_USERS` Kconfig option. Now the library expects at most one callback for each instance of the struct :c:struct:`bm_zms_fs`.
	* The :kconfig:option:`CONFIG_BM_ZMS_MAX_USERS` Kconfig option.
	Now, the library expects at most one callback for each instance of the struct :c:struct:`bm_zms_fs`.

Remember, every sentence on its own line. Do we need "at most"?

[MirkoCovizzi]

"At most" clarifies that the callback is optional. Without it, it would seem that there has to be one callback. It also clarifies the difference compared to before, where it would seem that a user can set multiple callbacks on an instance.

[peknis]

Suggested change

	* The :c:member:`bm_zms_init_flags.cb_registred` member since it was not used anymore.
	* The :c:member:`bm_zms_init_flags.cb_registred` member as it was not used anymore.

[peknis]

Approved with a couple of further suggestions.

[peknis]

Suggested change

	* The :kconfig:option:`CONFIG_BM_ZMS_MAX_USERS` Kconfig option.
	* The ``CONFIG_BM_ZMS_MAX_USERS`` Kconfig option.

Sorry, my bad. The :kconfig:option: would not work anymore as the option is removed, thus double back-ticks.

[peknis]

Suggested change

	* The :c:member:`bm_zms_init_flags.cb_registred` member as it was not used anymore.
	* The ``bm_zms_init_flags.cb_registred`` member as it was not used anymore.

Same here, the directive :c:member: would not work as it is removed.