Fix there being more security updates than total updates in notifier

Authored by Zren on Jun 19 2018, 12:26 AM.



We currently read the previous m_securityCount instead of using the current securityCount.

BUG: 392056
FIXED-IN 5.13.1 (or 5.13.2?)
FIXED-IN 5.12.6 (This should be backported right?)

The bug report is from 5.12.3, and was also reported on reddit today. Both have a screenshot of the bug.

We could also start the counter at 0 and add securityCount afterward.

-    uint count = securityUpdatesCount();
+    uint count = 0;
     foreach(BackendNotifierModule* module, m_backends)
         count += module->updatesCount();
+    count += securityCount;
Test Plan

I haven't attempted to compile this or test it. I don't have any security updates atm anyways.
I haven't pushed a bugfix to a Plasma/5.__ branch before either. Do I make a separate commit for 5.12, 5.13, and master?

Diff Detail

R134 Discover Software Store
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.
Zren created this revision.Jun 19 2018, 12:26 AM
Restricted Application added a project: Plasma. · View Herald TranscriptJun 19 2018, 12:26 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
Zren requested review of this revision.Jun 19 2018, 12:26 AM
ngraham added subscribers: apol, ngraham.

For bugfixes like this, the workflow is to commit to the "stable branch" (in this case Plasma/5.12) and then merge to master. In this case you would also merge to Plasma/5.13. the FIXED-IN tag should be 5.12.6.

See also

As for the patch, I tried it out and it seems to solve the problem! No opinion on whether your current approach or the alternative one you mentioned would be most appropriate. I'll let @apol have the final say.

Do I make a separate commit for 5.12, 5.13, and master?

If possible, we commit to the oldest relevant branch then merge upwards.

commit to 5.12
merge 5.12 into 5.13
merge 5.13 into master

Otherwise git will treat them as different commits which makes some admin tasks, like seeing which branches contain a fix, harder.

apol accepted this revision.Jun 19 2018, 2:12 PM

Please land in 5.12.

This revision is now accepted and ready to land.Jun 19 2018, 2:12 PM
apol added a comment.Jun 19 2018, 2:13 PM

You just commit it to Plasma/5.12 branch and it will get propagated when I merge 5.12 to 5.13 and master

This revision was automatically updated to reflect the committed changes.
Zren added a comment.Jun 19 2018, 9:22 PM

Thanks @apol for merging it. Thanks @ngraham for testing it and the link and @davidedmundson for explaining how bugfixes are merged.