allow string.c to be disabled when it conflicts with other libs

[liebman]

There are other crates that are based on C libs and define various functions like strchr. This PR adds a feature that when enabled does not compile string.c and expects some external source to provide the functions within.

I ran into this issue with the current (unreleased) esp-wifi that when combined with littlefs2 fails to link because strchr is multiply defined.

When combining crates that do this one can disable string.c and if needed add this to fill in any functions that are still missing:

tinyrlibc = { version = "0.4.0", features = ["strspn", "strcspn", "strlen"] }

[robin-nitrokey]

Having features that disable code is somewhat counter-intuitive for me. But I’m not sure if a positive feature that enables strings.c would really be better here. Do you have a preference?

[liebman]

I have no preference - as long as there is control as to where these functions come from.

[sosthene-nitrokey]

From the Cargo book

A consequence of this is that features should be additive. That is, enabling a feature should not disable functionality, and it should usually be safe to enable any combination of features. A feature should not introduce a SemVer-incompatible change.

Another solution would be to expect the user to always bring their own implementation. This is only matters for embedded so it's not shocking.

[liebman]

Another solution would be to expect the user to always bring their own implementation. This is only matters for embedded so it's not shocking.

Thats fine too. One can always use

tinyrlibc = { version = "0.4.0", features = ["strspn", "strcspn", "strlen"] }

to bridge the gap.

[robin-nitrokey]

I don’t see strspn and strcspn in tinyrlibc v0.4.0. Am I missing something?

[liebman]

Your correct :-(

[robin-nitrokey]

In that case I would vote for keeping the functions in littlefs2-sys but use the feature flag to enable them (instead of disabling them if the feature is set) – mabye just call it string-funcs?

[liebman]

That works for me.

[sosthene-nitrokey]

I'll open an issue on tinyrlibc, honestly being able to get rid of theses implementation seems good (and using a rust implementation instead too).

[robin-nitrokey]

tinyrlibc v0.5.0 provides the missing functions (rust-embedded-community/tinyrlibc#33) so I think we can go forward with removing them from littlefs2-sys.

[sosthene-nitrokey]

I would still keep a feature flag that brings in tinyrlibc as a dependency to still have the two functions.