Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add optional file metadata #128

Merged
merged 2 commits into from
Jan 20, 2017
Merged

Add optional file metadata #128

merged 2 commits into from
Jan 20, 2017

Conversation

rojer
Copy link
Contributor

@rojer rojer commented Jan 20, 2017

Metadata is stored in the file index structure and is opaque to SPIFFS.
It allows user to associate additional information with a file.

Fixes #127

Metadata is stored in the file index structure and is opaque to SPIFFS.
It allows user to associate additional information with a file.

Fixes #127
Copy link
Owner

@pellepl pellepl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can tell, it is flawless, but for one change in a comment. Good work!

@@ -107,6 +107,13 @@
#define SPIFFS_OBJ_NAME_LEN (32)
#endif

// Maximum length of the metadata associated with an object.
// Setting to non-zero value enables metadata-related API but also
// changes the on-disk format, so the change is not backward-compatible.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add following to this comment:
//
// Do note: the meta length must never exceed
// logical_page_size - (SPIFFS_OBJ_NAME_LEN + 64)
//
// This is derived from following:
// logical_page_size - (SPIFFS_OBJ_NAME_LEN + sizeof(spiffs_page_header) +
// spiffs_object_ix_header fields + + at least some LUT entries)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Owner

@pellepl pellepl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this. I will meddle a bit with it soon enough, but will not change current API or functionality. Cheers!

@pellepl pellepl merged commit 5d4d1a4 into pellepl:master Jan 20, 2017
@rojer
Copy link
Contributor Author

rojer commented Jan 20, 2017

thanks for the speedy review!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants