Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

add entry message wide string support #412

Merged
merged 3 commits into from
Apr 7, 2024
Merged

add entry message wide string support #412

merged 3 commits into from
Apr 7, 2024

Conversation

goatshriek
Copy link
Owner

Implements stumpless_set_entry_message_str_w and fixes some wide string functionality as well.

This is a continuation of #376, which implemented most of this functionality but had a few problems remaining, and has since gone stale.

This fixes #272, which can be referenced for more information.

@goatshriek goatshriek self-assigned this Mar 30, 2024
Copy link

codecov bot commented Mar 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.91%. Comparing base (aef696d) to head (eed1e65).

Additional details and impacted files
@@            Coverage Diff             @@
##           latest     #412      +/-   ##
==========================================
+ Coverage   90.61%   90.91%   +0.29%     
==========================================
  Files          47       47              
  Lines        4307     4326      +19     
  Branches      572      574       +2     
==========================================
+ Hits         3903     3933      +30     
+ Misses        278      264      -14     
- Partials      126      129       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@goatshriek
Copy link
Owner Author

@rmknan, this pull is the completion of your previous one. Please feel free to provide a review of it, particularly since it is almost entirely derived from your work. I'll leave it open for a week to give you some time to comment before merging it in.

Copy link

sonarcloud bot commented Mar 30, 2024

@goatshriek goatshriek added the enhancement new features or improvements label Mar 30, 2024
@@ -887,6 +887,7 @@ stumpless_set_entry_message_str_w( struct stumpless_entry *entry,
if( !new_message ){
return NULL;
}
new_message_size--; // leave off the NULL character
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I am going through the commits. Could you tell me about this line ?

Copy link
Owner Author

Choose a reason for hiding this comment

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

The wide string conversion functions return a size that includes the NULL terminator, but the entry message_length member shouldn't include the NULL terminator in it. Stumpless currently assumes that this string is NULL-terminated though, and specifically the stumpless_get_entry_message function does when copying out the message to return. If the NULL terminator is left in on this length, the get entry function will try to read an extra byte at the end because it expects a NULL terminator to be there, which is invalid (and causes valgrind errors).

It's a confusing convention, and the next major version (v3.0) will drop the NULL terminators completely for this reason, but for now this line is necessary to keep everything consistent.

@goatshriek goatshriek merged commit b81808e into latest Apr 7, 2024
86 checks passed
@goatshriek goatshriek deleted the message-wstring branch April 7, 2024 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement new features or improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

create wide string set entry message functions
2 participants