Merge lp://qastaging/~jdstrand/snap-confine/fix-udev-for-1604 into lp://qastaging/~snappy-dev/snap-confine/trunk

Thanks Leo, pushed in r123.

See inline comments.

I'll also note that, due to release deadlines, I did not have sufficient time to review the greater design changes that are occurring due to this MP. I really only had a chance to review the code changes.

>> +const char *static_devices[] = {
>
> I may be missing something but I don't see why this array was lifted from the function that uses it and turned into a global. If it is to remain as global, it should be declared as "static const char *static_devices[]" since it isn't used outside of main.c.

Ah, yes, this should have been moved back down after I made the code simpler.

> This string test is incomplete for a couple reasons:
>
> 1) tagname_len could be greater than the size of the tagname buffer
> 2) the tagname buffer could be unterminated

True. I was thinking about the must_snprintf() that was called to set it would have taken care of this, but if we are doing all these checks for future-proof coding, we should do all the checks.

>> Is it intentional that we don't attempt to setuid(0) (or fail?) if the first conditional is not met? It seems correct to continue on if real_uid is 0 but what about when effective_uid is not 0? I guess snappy-app-dev will simply fail due to insufficient perms and everything will be ok.

Yes, it will fail due to insufficient perms and yes this was intentional. The idea here is to raise if we can but not if we can't. The testsuite will have euid as non-zero, under sudo real_uid and euid are 0, and under suid launcher euid is 0 but real_uid is not.

Thanks for the review!