Mir

Merge lp://qastaging/~andreas-pokorny/mir/fix-null-cmdline-crash into lp://qastaging/mir

  1. fix-null-cmdline-crash
  2. Merge into development-branch
Proposed by Andreas Pokorny
Status: Rejected
Rejected by: Daniel van Vugt
Proposed branch: lp://qastaging/~andreas-pokorny/mir/fix-null-cmdline-crash
Merge into: lp://qastaging/mir
Diff against target: 37 lines (+15/-1)
2 files modified
src/platform/options/default_configuration.cpp (+3/-1)
tests/acceptance-tests/server_configuration_options.cpp (+12/-0)
To merge this branch: bzr merge lp://qastaging/~andreas-pokorny/mir/fix-null-cmdline-crash
Reviewer Review Type Date Requested Status
Alan Griffiths Disapprove
PS Jenkins bot (community) continuous-integration Approve
Daniel van Vugt Approve
Gerry Boland Pending
Review via email: mp+265936@code.qastaging.launchpad.net

Commit message

merged from 0.14.0: fix to avoid crash within qtmir test suite when accessing missing program parameter

qmir test suite passes an empty (argc == 0) command line to mir. Due to a change in configuration handling we access argv[0] on server construction.

Description of the change

This is one of the small changes that were necessary to release 0.14.0. This change allows providing argc=0 to Server. qtmir does that in a couple of tests.

To post a comment you must log in.
Revision history for this message
Daniel van Vugt (vanvugt) :
review: Approve
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

I realize it is low cost, but should we support this usecase? (Or fix qtmir?)

review: Needs Information
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:2783
http://jenkins.qa.ubuntu.com/job/mir-ci/4419/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/3315
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/mir-clang-ts-wily-amd64-build/168
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/840
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/3263
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/568
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/568/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3263
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3263/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/6059
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/22140

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/4419/rebuild

review: Approve (continuous-integration)
Revision history for this message
Kevin DuBois (kdub) wrote :

setting argc == 0 seems like a user error to me; just because it is well-known how argc/argv works.

I don't mind fixing though so that argc/argv work like a normal array, but while we're fixing zany user errors, we could also fix if someone does:
server.set_command_line(-112, nullptr);

Revision history for this message
Andreas Pokorny (andreas-pokorny) wrote :

.. the necessary change in qtmir would be small too. I made the change in mir because I believe that qtmirs test setup worked with previous versions of mir/

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

Given the choice between adding logic to production code just to support test code and fixing the test code not to need it...I recommend fixing the test code.

review: Disapprove
Revision history for this message
Cemil Azizoglu (cemil-azizoglu) wrote :

> Given the choice between adding logic to production code just to support test
> code and fixing the test code not to need it...I recommend fixing the test
> code.

+1

Unmerged revisions

2783. By Andreas Pokorny

merged from 0.14.0: fix to avoid crash within qtmir test suite

qmir test suite passes an empty (argc == 0) command line to mir. Due to a change in configuration handling we access argv[0] on server construction.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches