Differential D15715

remove own implementation of QString to TString conversion for taglibwriter
ClosedPublic
Actions

Authored by astippich on Sep 23 2018, 7:05 PM.

Details

Reviewers
mgallien
bruns
svuorela
Commits
R286:c413d83851c5: remove own implementation of QString to TString conversion for taglibwriter
Summary

as pointed out in D13700, the custom function for conversion of taglib string
to QString is unsafe and will break if size(wchar) == 4 bytes. Use the function
provided by taglib for the conversion to TString. See also D15614

Test Plan

tests pass

Diff Detail

Repository
R286 KFileMetaData
Branch
taglib_write_string
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 3161
Build 3179: arc lint + arc unit
astippich created this revision.Sep 23 2018, 7:05 PM
Restricted Application added projects: Frameworks, Baloo. · View Herald TranscriptSep 23 2018, 7:05 PM
Restricted Application added subscribers: Baloo, kde-frameworks-devel. · View Herald Transcript
astippich requested review of this revision.Sep 23 2018, 7:05 PM
astippich added a comment.Sep 23 2018, 7:07 PM

since I'm still unsure about those things: the q2t function was not declared static, but was never exported. It is still safe to remove, right?

svuorela added a comment.Sep 23 2018, 7:23 PM
In D15715#330743, @astippich wrote:

since I'm still unsure about those things: the q2t function was not declared static, but was never exported. It is still safe to remove, right?

Yes. not exported (and not declared in a header file). Safe.

bruns added a comment.Sep 27 2018, 10:21 PM

Can you copy the summary verbatim from D15614, and just add a "See also D15614"?
Otherwise, good to go.

bruns requested changes to this revision.Sep 27 2018, 10:35 PM
This revision now requires changes to proceed.Sep 27 2018, 10:35 PM
astippich edited the summary of this revision. (Show Details)Sep 28 2018, 7:58 PM
bruns added a comment.Sep 28 2018, 8:07 PM

Thx! Can you also add some line breaks at ~80 chars? ;-)

astippich added a comment.Sep 28 2018, 8:09 PM
In D15715#333567, @bruns wrote:

Thx! Can you also add some line breaks at ~80 chars? ;-)

sure :)

astippich edited the summary of this revision. (Show Details)Sep 28 2018, 8:10 PM
bruns accepted this revision.Sep 28 2018, 8:11 PM
This revision is now accepted and ready to land.Sep 28 2018, 8:11 PM
Closed by commit R286:c413d83851c5: remove own implementation of QString to TString conversion for taglibwriter (authored by astippich). · Explain WhySep 28 2018, 8:48 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Msrc/writers/taglibwriter.cpp (15 lines)

Diff 42202

View Options

src/writers/taglibwriter.cpp

Loading...