Merge lp://qastaging/~vicamo/upstart/xenial-escape-systemd-strings into lp://qastaging/~ubuntu-core-dev/upstart/xenial

Proposed by You-Sheng Yang
Status: Merged
Merged at revision: 1632
Proposed branch: lp://qastaging/~vicamo/upstart/xenial-escape-systemd-strings
Merge into: lp://qastaging/~ubuntu-core-dev/upstart/xenial
Diff against target: 83 lines (+51/-4)
1 file modified
extra/upstart-local-bridge.c (+51/-4)
To merge this branch: bzr merge lp://qastaging/~vicamo/upstart/xenial-escape-systemd-strings
Reviewer Review Type Date Requested Status
Łukasz Zemczak not an upstart maintainer Approve
Review via email: mp+307140@code.qastaging.launchpad.net

Description of the change

upstart-local-bridge: escape strings for systemd

Android property names and values may contain invalid characters for systemd and breaks the mechanism here.

To post a comment you must log in.
Revision history for this message
Łukasz Zemczak (sil2100) wrote :

In overall the proposed change looks fine an seems to work reasonably well. The same could have been realized without the new hexchar() function, but having it makes things potentially slightly faster and simplified.
That being said, I do not know the upstart code-base that well, so maybe it would be nice if some other maintainer took a quick look as well.

One stylistic recommendation: in the inline comments, the if, else if and else parts could use { } even in the single-line conditional commands. It's much cleaner this way and follows the upstart code style better.

review: Approve (not an upstart maintainer)
Revision history for this message
Łukasz Zemczak (sil2100) :
1633. By You-Sheng Yang

Address stylistic review comments.

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

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