Re: Review Request 125969: kinit: fix Coverity issues + small optimization

2015-11-06 Thread Nick Shaforostoff 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125969/
---

(Updated Nov. 7, 2015, 12:04 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Alex Merry.


Changes
---

Submitted with commit 04e64f8ec47476530157cefc434776f4aa93a27f by Nick 
Shaforostoff to branch master.


Repository: kinit


Description
---

this patch fixes two coverity issues ranked 'outstanding':
https://scan5.coverity.com/reports.htm#v39099/p10103/fileInstanceId=82663767=24554334=258481
https://scan5.coverity.com/reports.htm#v39099/p10103/fileInstanceId=82663767=24557588=258474

and also does small string-related optimization by eliminating redundant 
mallocs done by QByteArray ctor


Diffs
-

  src/kdeinit/kinit.cpp 9e775b6 

Diff: https://git.reviewboard.kde.org/r/125969/diff/


Testing
---

compiles fine


Thanks,

Nick Shaforostoff

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 125969: kinit: fix Coverity issues + small optimization

2015-11-06 Thread Alex Merry 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125969/#review88106
---

Ship it!


Ship It!

- Alex Merry


On Nov. 6, 2015, 4 p.m., Nick Shaforostoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125969/
> ---
> 
> (Updated Nov. 6, 2015, 4 p.m.)
> 
> 
> Review request for KDE Frameworks and Alex Merry.
> 
> 
> Repository: kinit
> 
> 
> Description
> ---
> 
> this patch fixes two coverity issues ranked 'outstanding':
> https://scan5.coverity.com/reports.htm#v39099/p10103/fileInstanceId=82663767=24554334=258481
> https://scan5.coverity.com/reports.htm#v39099/p10103/fileInstanceId=82663767=24557588=258474
> 
> and also does small string-related optimization by eliminating redundant 
> mallocs done by QByteArray ctor
> 
> 
> Diffs
> -
> 
>   src/kdeinit/kinit.cpp 9e775b6 
> 
> Diff: https://git.reviewboard.kde.org/r/125969/diff/
> 
> 
> Testing
> ---
> 
> compiles fine
> 
> 
> Thanks,
> 
> Nick Shaforostoff
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 125969: kinit: fix Coverity issues + small optimization

2015-11-06 Thread Nick Shaforostoff 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125969/
---

(Updated Nov. 6, 2015, 4 p.m.)


Review request for KDE Frameworks and Alex Merry.


Changes
---

now displayEnvVarName_c() returns const char* and displayEnvVarName() is 
wrapping it into a QByteArray


Repository: kinit


Description
---

this patch fixes two coverity issues ranked 'outstanding':
https://scan5.coverity.com/reports.htm#v39099/p10103/fileInstanceId=82663767=24554334=258481
https://scan5.coverity.com/reports.htm#v39099/p10103/fileInstanceId=82663767=24557588=258474

and also does small string-related optimization by eliminating redundant 
mallocs done by QByteArray ctor


Diffs (updated)
-

  src/kdeinit/kinit.cpp 9e775b6 

Diff: https://git.reviewboard.kde.org/r/125969/diff/


Testing
---

compiles fine


Thanks,

Nick Shaforostoff

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 125969: kinit: fix Coverity issues + small optimization

2015-11-05 Thread Nick Shaforostoff 


> On Nov. 5, 2015, 11:37 p.m., Alex Merry wrote:
> > src/kdeinit/kinit.cpp, line 370
> > 
> >
> > Why call fromRawData with an explicit strlen()? 
> > QByteArray(displayEnvVarName()) will have the same effect.

fromRawData doesn't copy the data provided. the deep copy will be made later by 
operator+ into a properly sized qbytearray to store "="-ending string


- Nick


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125969/#review88068
---


On Nov. 5, 2015, 10:16 p.m., Nick Shaforostoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125969/
> ---
> 
> (Updated Nov. 5, 2015, 10:16 p.m.)
> 
> 
> Review request for KDE Frameworks and Alex Merry.
> 
> 
> Repository: kinit
> 
> 
> Description
> ---
> 
> this patch fixes two coverity issues ranked 'outstanding':
> https://scan5.coverity.com/reports.htm#v39099/p10103/fileInstanceId=82663767=24554334=258481
> https://scan5.coverity.com/reports.htm#v39099/p10103/fileInstanceId=82663767=24557588=258474
> 
> and also does small string-related optimization by eliminating redundant 
> mallocs done by QByteArray ctor
> 
> 
> Diffs
> -
> 
>   src/kdeinit/kinit.cpp 9e775b6 
> 
> Diff: https://git.reviewboard.kde.org/r/125969/diff/
> 
> 
> Testing
> ---
> 
> compiles fine
> 
> 
> Thanks,
> 
> Nick Shaforostoff
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 125969: kinit: fix Coverity issues + small optimization

2015-11-05 Thread Alex Merry 


> On Nov. 5, 2015, 11:37 p.m., Alex Merry wrote:
> > src/kdeinit/kinit.cpp, line 370
> > 
> >
> > Why call fromRawData with an explicit strlen()? 
> > QByteArray(displayEnvVarName()) will have the same effect.
> 
> Nick Shaforostoff wrote:
> fromRawData doesn't copy the data provided. the deep copy will be made 
> later by operator+ into a properly sized qbytearray to store "="-ending string

Ah, ok, makes sense. It would be a bit nicer to wrap up getting 
displayEnvVarName() as a QByteArray in a function that was next to the 
definition of displayEnvVarName() in the file - given the lifetime constraints 
of fromRawData, it's nice to be able to see easily from where it is used that 
those constraints will be satisfied.


- Alex


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125969/#review88068
---


On Nov. 5, 2015, 10:16 p.m., Nick Shaforostoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125969/
> ---
> 
> (Updated Nov. 5, 2015, 10:16 p.m.)
> 
> 
> Review request for KDE Frameworks and Alex Merry.
> 
> 
> Repository: kinit
> 
> 
> Description
> ---
> 
> this patch fixes two coverity issues ranked 'outstanding':
> https://scan5.coverity.com/reports.htm#v39099/p10103/fileInstanceId=82663767=24554334=258481
> https://scan5.coverity.com/reports.htm#v39099/p10103/fileInstanceId=82663767=24557588=258474
> 
> and also does small string-related optimization by eliminating redundant 
> mallocs done by QByteArray ctor
> 
> 
> Diffs
> -
> 
>   src/kdeinit/kinit.cpp 9e775b6 
> 
> Diff: https://git.reviewboard.kde.org/r/125969/diff/
> 
> 
> Testing
> ---
> 
> compiles fine
> 
> 
> Thanks,
> 
> Nick Shaforostoff
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 125969: kinit: fix Coverity issues + small optimization

2015-11-05 Thread Alex Merry 

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125969/#review88068
---


Generally makes sense to me, other than my first note below:


src/kdeinit/kinit.cpp (line 370)


Why call fromRawData with an explicit strlen()? 
QByteArray(displayEnvVarName()) will have the same effect.



src/kdeinit/kinit.cpp (lines 1076 - 1077)


This whole thing is unpleasantly C-ish, and a QByteArray or QVector 
would have been altogether nicer, but that would be a much bigger refactoring 
of the code.


- Alex Merry


On Nov. 5, 2015, 10:16 p.m., Nick Shaforostoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125969/
> ---
> 
> (Updated Nov. 5, 2015, 10:16 p.m.)
> 
> 
> Review request for KDE Frameworks and Alex Merry.
> 
> 
> Repository: kinit
> 
> 
> Description
> ---
> 
> this patch fixes two coverity issues ranked 'outstanding':
> https://scan5.coverity.com/reports.htm#v39099/p10103/fileInstanceId=82663767=24554334=258481
> https://scan5.coverity.com/reports.htm#v39099/p10103/fileInstanceId=82663767=24557588=258474
> 
> and also does small string-related optimization by eliminating redundant 
> mallocs done by QByteArray ctor
> 
> 
> Diffs
> -
> 
>   src/kdeinit/kinit.cpp 9e775b6 
> 
> Diff: https://git.reviewboard.kde.org/r/125969/diff/
> 
> 
> Testing
> ---
> 
> compiles fine
> 
> 
> Thanks,
> 
> Nick Shaforostoff
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel