[PATCH] trivial: remove unneeded int

[PATCH] trivial: remove unneeded int

[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;
 }
 
 /*

Re: [PATCH] trivial: remove unneeded int

[Petr Lautrbach]

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

Re: [PATCH] trivial: remove unneeded int

[Petr Lautrbach]

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 

Re: [PATCH] trivial: remove unneeded int

[Jokke Hämäläinen]

[-- Attachment #1: Type: text/plain, Size: 405 bytes --]


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

Re: [PATCH] trivial: remove unneeded int

[William Roberts]

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

Re: [PATCH] trivial: remove unneeded int

[Jokke Hämäläinen]

[-- Attachment #1: Type: text/plain, Size: 305 bytes --]


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