| Message ID | alpine.LFD.2.21.1905242306150.9521@34-41-5D-CA-59-C7 (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | trivial: remove unneeded int | expand |
Jokke Hämäläinen <jokke.hamalainen@kolttonen.fi> writes: > diff --git a/libsepol/src/context.c b/libsepol/src/context.c > index a88937fc..e81b28c6 100644 > --- a/libsepol/src/context.c > +++ b/libsepol/src/context.c > @@ -38,7 +38,6 @@ int context_is_valid(const policydb_t * p, const context_struct_t * c) > role_datum_t *role; > user_datum_t *usrdatum; > ebitmap_t types, roles; > - int ret = 1; > > ebitmap_init(&types); > ebitmap_init(&roles); > @@ -75,7 +74,7 @@ int context_is_valid(const policydb_t * p, const context_struct_t * c) > if (!mls_context_isvalid(p, c)) > return 0; > > - return ret; > + return 1; > } > > /* ack Would you please add Signed-off line (git commit -s ...) to the description, see `git log` and https://developercertificate.org/ Also, if the patch applies only to one subdirectory it's a common practice to use prefix in the subject of the patch, e.g.: libsepol: trivial: remove unneeded int Petr
Petr Lautrbach <plautrba@redhat.com> writes: > Jokke Hämäläinen <jokke.hamalainen@kolttonen.fi> writes: > >> diff --git a/libsepol/src/context.c b/libsepol/src/context.c >> index a88937fc..e81b28c6 100644 >> --- a/libsepol/src/context.c >> +++ b/libsepol/src/context.c >> @@ -38,7 +38,6 @@ int context_is_valid(const policydb_t * p, const context_struct_t * c) >> role_datum_t *role; >> user_datum_t *usrdatum; >> ebitmap_t types, roles; >> - int ret = 1; >> >> ebitmap_init(&types); >> ebitmap_init(&roles); >> @@ -75,7 +74,7 @@ int context_is_valid(const policydb_t * p, const context_struct_t * c) >> if (!mls_context_isvalid(p, c)) >> return 0; >> >> - return ret; >> + return 1; >> } >> >> /* > > ack > > Would you please add Signed-off line (git commit -s ...) to the description, see > `git log` and https://developercertificate.org/ > > Also, if the patch applies only to one subdirectory it's a common practice > to use prefix in the subject of the patch, e.g.: > > libsepol: trivial: remove unneeded int > The same comment applies also to your other unmerged patches * trivial: remove unneeded int * remove redundant if * More accurate error messages * trivial fix incorrect indentation * trivial typo fix travis CI seems to be happy with all of them https://travis-ci.org/bachradsusi/SELinuxProject-selinux/builds/542793341
Hello, On Fri, 7 Jun 2019, Petr Lautrbach wrote: > The same comment applies also to your other unmerged patches > > * trivial: remove unneeded int > * remove redundant if > * More accurate error messages > * trivial fix incorrect indentation > * trivial typo fix The simple patches now have a "Signed-off-by:" line in the description. Pull requests have been done. Best regards, Jokke Hämäläinen
On Wed, Jun 12, 2019 at 11:01 AM Jokke Hämäläinen <jokke.hamalainen@kolttonen.fi> wrote: > > > Hello, > > On Fri, 7 Jun 2019, Petr Lautrbach wrote: > > > The same comment applies also to your other unmerged patches > > > > * trivial: remove unneeded int > > * remove redundant if > > * More accurate error messages > > * trivial fix incorrect indentation > > * trivial typo fix > > The simple patches now have a "Signed-off-by:" line in > the description. > > Pull requests have been done. > You have all your new patches as individual PR's with merge commits, very weird: https://github.com/SELinuxProject/selinux/pull/154/commits Rather then: 1. creating a PR per patch 2. merge workflow Please: 1. Since all your patches are related in goal within the series, just put them on 1 PR 2. rebase workflow, ie always git pull --rebase origin/master I don't know if this is the most efficient way with git, but you can just cherry-pick all those commits on a branch that is up-to-date with master. 3. You still need to send them back out to the list 4. commit messages lacking On top of that, Stephen has mentioned that we don't want to get in the habit of taking style fixes. The three actual changes you have are: > > * trivial: remove unneeded int > > * remove redundant if > > * More accurate error messages Additionally, lets work on those commit messages, typically when someone starts with <label>: that label isn't something like trivial, it's used for referencing the component touched. libsepol: remove unneeded int the routine context_is_valid() had an int that was used as a return code that was set and never modified. Just return the constant instead. <signed-off...> I'm OK with those three if you fix the 4 issues I pointed out. I'll let Stephen decide the fate on the pure style patches. > Best regards, > Jokke Hämäläinen
Hi! On Wed, 12 Jun 2019, William Roberts wrote: > I'm OK with those three if you fix the 4 issues I pointed out. I'll > let Stephen decide the fate on the pure style patches. Sorry for this mess, guys. I will resend the three patches, hopefully correctly this time. Best regards, Jokke Hämäläinen
diff --git a/libsepol/src/context.c b/libsepol/src/context.c index a88937fc..e81b28c6 100644 --- a/libsepol/src/context.c +++ b/libsepol/src/context.c @@ -38,7 +38,6 @@ int context_is_valid(const policydb_t * p, const context_struct_t * c) role_datum_t *role; user_datum_t *usrdatum; ebitmap_t types, roles; - int ret = 1; ebitmap_init(&types); ebitmap_init(&roles); @@ -75,7 +74,7 @@ int context_is_valid(const policydb_t * p, const context_struct_t * c) if (!mls_context_isvalid(p, c)) return 0; - return ret; + return 1; } /*