<div dir="ltr"><div>Hello everybody,</div><div><br></div><div>While reviewing this change adding display for Tags in dive list view (<a href="https://github.com/Subsurface-divelog/subsurface/pull/1184">https://github.com/Subsurface-divelog/subsurface/pull/1184</a>), Lubomir and Dirk raised a concern regarding tag text that can exceed inputted buffer size. Issue comes from implementation/signature of taglist_get_tagstring function <a href="https://github.com/Subsurface-divelog/subsurface/blob/master/core/dive.c#L3023">https://github.com/Subsurface-divelog/subsurface/blob/master/core/dive.c#L3023</a> Certainly no current user has been using enough tags to actually encounter problems with this but who knows...</div><div><br></div><div>Staring from their input and doing some code digging, here is a short overview of my findings and proposals.</div><div><br></div><div><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">taglist_get_tagstring</span> function currently prints as many full tags as could be fitted in the inputted buffer and returns the length of the printed string. This approach raise several issues when it comes to long tag lists:</div><div> a) It is not possible to know from the returned information if all tags were actually printed.</div><div> b) When a user would (in the UI) add a long tag list, the full tag list is (at first) indeed saved. Then, only text returned by <span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">taglist_get_tagstring </span>gets displayed. This causes later edits of the tags to delete tags that were not displayed, not good... </div><div> c) Looking at DiveObjectHelper class the function is used with 256 buffer size so issue would become more visible if DiveObjectHelper::tags was actually used. It seems this method is not used yet (neither in Desktop nor in Mobile app) but I may be wrong, please point me to its usage if you know one...</div><div><br></div><div>Lubomir mentioned he could look into this 'issue' but did not have much free time, since I do have some on my side I can look into this change. I do prefer to consult the community before doing it though. Here are the different solutions that came to my mind:</div><div><br></div><div> 1) Add a output parameter to last printed struct tag_entry* in the list to the current <span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">taglist_get_tagstring allowing callers to iterate when needed (not my favorite)</span></div><div><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"> 2) It seems strange for a UI specific function to be part of dive.h/.c I would rather move this functionality to 'Qt level' say in qthelper.h/.c (or other better location if one of you have a better proposal) and implement it using QStrings (avoiding the pre-allocated buffer issue). This is already what is done for get_gas_string for example. It is my preferred proposal since all usage I could find of <span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">taglist_get_tagstring are in Qt code and this function is purely UI related code...</span></span></div><div><br></div><div>I do have other topics to discuss with you guys but I'll send separate emails not to mix things up.</div><div><br></div><div>Cheers,</div><div><br></div><div>Jeremie</div></div>