[OpenBSD] sic pledge patch

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[OpenBSD] sic pledge patch

Ali H. Fardan
Greetings ports@
I have gone through the code of sic (the irc client), and I have
successfully
pledged it, can this patch be merged to the OpenBSD ports version of
sic?

Raiz


diff --git a/sic.c b/sic.c
index ce6d216..695fd52 100644
--- a/sic.c
+++ b/sic.c
@@ -182,7 +182,14 @@ main(int argc, char *argv[]) {
  setbuf(stdout, NULL);
  setbuf(srv, NULL);
  setbuf(stdin, NULL);
- for(;;) { /* main loop */
+
+#ifdef __OpenBSD__
+#include <err.h>
+ if (pledge("stdio", NULL) == -1)
+ err(1, "pledge");
+#endif
+
+ for(;;) {
  FD_ZERO(&rd);
  FD_SET(0, &rd);
  FD_SET(fileno(srv), &rd);

Reply | Threaded
Open this post in threaded view
|

Re: [OpenBSD] sic pledge patch

Jeremie Courreges-Anglas-2

If you want to do contributions to the ports tree, remember that many
ports have a maintainer.  This maintainer should either be contacted
first or Cc'ed when sending a patch to ports.

Comments and patch proposal below.

"Ali H. Fardan" <[hidden email]> writes:

> Greetings ports@
> I have gone through the code of sic (the irc client), and I have
> successfully
> pledged it, can this patch be merged to the OpenBSD ports version of
> sic?

Probably, but not as is.

> Raiz
>
>
> diff --git a/sic.c b/sic.c
> index ce6d216..695fd52 100644
> --- a/sic.c
> +++ b/sic.c
> @@ -182,7 +182,14 @@ main(int argc, char *argv[]) {
>   setbuf(stdout, NULL);
>   setbuf(srv, NULL);
>   setbuf(stdin, NULL);
> - for(;;) { /* main loop */
> +
> +#ifdef __OpenBSD__

This checks has no value for the ports tree, and it would do more harm
than good, should upstream accept the patch as is.  What if pledge(2)
becomes available on another OS?

> +#include <err.h>

I assume that you wanted to keep the patch short, but #includ'ing
a header here looks weird to me.

> + if (pledge("stdio", NULL) == -1)
> + err(1, "pledge");
> +#endif
> +
> + for(;;) {
>   FD_ZERO(&rd);
>   FD_SET(0, &rd);
>   FD_SET(fileno(srv), &rd);
>

Here is an simplified diff to make use of pledge.  A quick look at the
main loop in sic.c makes me think that you are correct about the pledge
request.  My testing went only as far as faking a pledge(2) failure and
checking that the error message is correctly printed.

I'll leave the decision to Joerg.


Index: Makefile
===================================================================
RCS file: /cvs/ports/net/sic/Makefile,v
retrieving revision 1.5
diff -u -p -r1.5 Makefile
--- Makefile 19 Oct 2013 09:49:51 -0000 1.5
+++ Makefile 12 Aug 2016 20:01:40 -0000
@@ -3,6 +3,7 @@
 COMMENT= simple irc client
 
 DISTNAME= sic-1.2
+REVISION= 0
 
 CATEGORIES= net
 
@@ -13,6 +14,7 @@ MAINTAINER = Joerg Jung <[hidden email]
 # MIT/X
 PERMIT_PACKAGE_CDROM= Yes
 
+# uses pledge()
 WANTLIB= c
 
 MASTER_SITES= http://dl.suckless.org/tools/
Index: patches/patch-sic_c
===================================================================
RCS file: patches/patch-sic_c
diff -N patches/patch-sic_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-sic_c 12 Aug 2016 20:01:40 -0000
@@ -0,0 +1,12 @@
+$OpenBSD$
+--- sic.c.orig Fri Aug 12 21:45:07 2016
++++ sic.c Fri Aug 12 21:54:39 2016
+@@ -173,6 +173,8 @@ main(int argc, char *argv[]) {
+ fflush(srv);
+ setbuf(stdout, NULL);
+ setbuf(srv, NULL);
++ if (pledge("stdio", NULL) == -1)
++ eprint("error: pledge:");
+ for(;;) { /* main loop */
+ FD_ZERO(&rd);
+ FD_SET(0, &rd);


--
jca | PGP: 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Reply | Threaded
Open this post in threaded view
|

Re: [OpenBSD] sic pledge patch

Ali H. Fardan
On 2016-08-12 23:03, [hidden email] wrote:
> If you want to do contributions to the ports tree, remember that many
> ports have a maintainer.  This maintainer should either be contacted
> first or Cc'ed when sending a patch to ports.

apologies

>
> Probably, but not as is.
>

of course

> This checks has no value for the ports tree, and it would do more harm
> than good, should upstream accept the patch as is.  What if pledge(2)
> becomes available on another OS?

if pledge became available on another OS, it would be their job to use
this patch, also I wrote the #ifdef because I intended to submit this
patch to the mainstream sic, but I changed my mind and I thought that
this is the correct place to do it, so it is not necessary to include
it.

Reply | Threaded
Open this post in threaded view
|

Re: [OpenBSD] sic pledge patch

Jeremie Courreges-Anglas-2
"Ali H. Fardan" <[hidden email]> writes:

> On 2016-08-12 23:03, [hidden email] wrote:
>> If you want to do contributions to the ports tree, remember that many
>> ports have a maintainer.  This maintainer should either be contacted
>> first or Cc'ed when sending a patch to ports.
>
> apologies
>
>>
>> Probably, but not as is.
>>
>
> of course
>
>> This checks has no value for the ports tree, and it would do more harm
>> than good, should upstream accept the patch as is.  What if pledge(2)
>> becomes available on another OS?
>
> if pledge became available on another OS, it would be their job to use
> this patch,

From a general POV, if the point of the patch we include in the ports
tree is to be pushed upstream, I don't see why the use of pledge(2)
wouldn't be as automatic as possible if available, just like for any
other function.

> also I wrote the #ifdef because I intended to submit this
> patch to the mainstream sic, but I changed my mind and I thought that
> this is the correct place to do it, so it is not necessary to include
> it.

Joerg knows better than us whether the use of pledge should be pushed
upstream. :)

--
jca | PGP: 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Reply | Threaded
Open this post in threaded view
|

Re: [OpenBSD] sic pledge patch

Stuart Henderson
In reply to this post by Jeremie Courreges-Anglas-2
On 2016/08/12 22:03, Jeremie Courreges-Anglas wrote:

>
> If you want to do contributions to the ports tree, remember that many
> ports have a maintainer.  This maintainer should either be contacted
> first or Cc'ed when sending a patch to ports.
>
> Comments and patch proposal below.
>
> "Ali H. Fardan" <[hidden email]> writes:
>
> > Greetings ports@
> > I have gone through the code of sic (the irc client), and I have
> > successfully
> > pledged it, can this patch be merged to the OpenBSD ports version of
> > sic?
>
> Probably, but not as is.
>
> > Raiz
> >
> >
> > diff --git a/sic.c b/sic.c
> > index ce6d216..695fd52 100644
> > --- a/sic.c
> > +++ b/sic.c
> > @@ -182,7 +182,14 @@ main(int argc, char *argv[]) {
> >   setbuf(stdout, NULL);
> >   setbuf(srv, NULL);
> >   setbuf(stdin, NULL);
> > - for(;;) { /* main loop */
> > +
> > +#ifdef __OpenBSD__
>
> This checks has no value for the ports tree, and it would do more harm
> than good, should upstream accept the patch as is.  What if pledge(2)
> becomes available on another OS?
>
> > +#include <err.h>
>
> I assume that you wanted to keep the patch short, but #includ'ing
> a header here looks weird to me.
>
> > + if (pledge("stdio", NULL) == -1)
> > + err(1, "pledge");
> > +#endif
> > +
> > + for(;;) {
> >   FD_ZERO(&rd);
> >   FD_SET(0, &rd);
> >   FD_SET(fileno(srv), &rd);
> >
>
> Here is an simplified diff to make use of pledge.  A quick look at the
> main loop in sic.c makes me think that you are correct about the pledge
> request.  My testing went only as far as faking a pledge(2) failure and
> checking that the error message is correctly printed.
>
> I'll leave the decision to Joerg.
>
>
> Index: Makefile
> ===================================================================
> RCS file: /cvs/ports/net/sic/Makefile,v
> retrieving revision 1.5
> diff -u -p -r1.5 Makefile
> --- Makefile 19 Oct 2013 09:49:51 -0000 1.5
> +++ Makefile 12 Aug 2016 20:01:40 -0000
> @@ -3,6 +3,7 @@
>  COMMENT= simple irc client
>  
>  DISTNAME= sic-1.2
> +REVISION= 0
>  
>  CATEGORIES= net
>  
> @@ -13,6 +14,7 @@ MAINTAINER = Joerg Jung <[hidden email]
>  # MIT/X
>  PERMIT_PACKAGE_CDROM= Yes
>  
> +# uses pledge()
>  WANTLIB= c
>  
>  MASTER_SITES= http://dl.suckless.org/tools/
> Index: patches/patch-sic_c
> ===================================================================
> RCS file: patches/patch-sic_c
> diff -N patches/patch-sic_c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-sic_c 12 Aug 2016 20:01:40 -0000
> @@ -0,0 +1,12 @@
> +$OpenBSD$
> +--- sic.c.orig Fri Aug 12 21:45:07 2016
> ++++ sic.c Fri Aug 12 21:54:39 2016
> +@@ -173,6 +173,8 @@ main(int argc, char *argv[]) {
> + fflush(srv);
> + setbuf(stdout, NULL);
> + setbuf(srv, NULL);
> ++ if (pledge("stdio", NULL) == -1)
> ++ eprint("error: pledge:");
> + for(;;) { /* main loop */
> + FD_ZERO(&rd);
> + FD_SET(0, &rd);
>
>
> --
> jca | PGP: 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE
>

This looks like the correct diff for ports to me.

Reply | Threaded
Open this post in threaded view
|

Re: [OpenBSD] sic pledge patch

Joerg Jung
In reply to this post by Jeremie Courreges-Anglas-2
On Fri, Aug 12, 2016 at 10:03:46PM +0200, Jeremie Courreges-Anglas wrote:

>
> If you want to do contributions to the ports tree, remember that many
> ports have a maintainer.  This maintainer should either be contacted
> first or Cc'ed when sending a patch to ports.
>
> Comments and patch proposal below.
>
> "Ali H. Fardan" <[hidden email]> writes:
>
> > Greetings ports@
> > I have gone through the code of sic (the irc client), and I have
> > successfully
> > pledged it, can this patch be merged to the OpenBSD ports version of
> > sic?
>
> Probably, but not as is.
>
> > Raiz
> >
> >
> > diff --git a/sic.c b/sic.c
> > index ce6d216..695fd52 100644
> > --- a/sic.c
> > +++ b/sic.c
> > @@ -182,7 +182,14 @@ main(int argc, char *argv[]) {
> >   setbuf(stdout, NULL);
> >   setbuf(srv, NULL);
> >   setbuf(stdin, NULL);
> > - for(;;) { /* main loop */
> > +
> > +#ifdef __OpenBSD__
>
> This checks has no value for the ports tree, and it would do more harm
> than good, should upstream accept the patch as is.  What if pledge(2)
> becomes available on another OS?
>
> > +#include <err.h>
>
> I assume that you wanted to keep the patch short, but #includ'ing
> a header here looks weird to me.
>
> > + if (pledge("stdio", NULL) == -1)
> > + err(1, "pledge");
> > +#endif
> > +
> > + for(;;) {
> >   FD_ZERO(&rd);
> >   FD_SET(0, &rd);
> >   FD_SET(fileno(srv), &rd);
> >
>
> Here is an simplified diff to make use of pledge.  A quick look at the
> main loop in sic.c makes me think that you are correct about the pledge
> request.  My testing went only as far as faking a pledge(2) failure and
> checking that the error message is correctly printed.
>
> I'll leave the decision to Joerg.

ok jung@
 

> Index: Makefile
> ===================================================================
> RCS file: /cvs/ports/net/sic/Makefile,v
> retrieving revision 1.5
> diff -u -p -r1.5 Makefile
> --- Makefile 19 Oct 2013 09:49:51 -0000 1.5
> +++ Makefile 12 Aug 2016 20:01:40 -0000
> @@ -3,6 +3,7 @@
>  COMMENT= simple irc client
>  
>  DISTNAME= sic-1.2
> +REVISION= 0
>  
>  CATEGORIES= net
>  
> @@ -13,6 +14,7 @@ MAINTAINER = Joerg Jung <[hidden email]
>  # MIT/X
>  PERMIT_PACKAGE_CDROM= Yes
>  
> +# uses pledge()
>  WANTLIB= c
>  
>  MASTER_SITES= http://dl.suckless.org/tools/
> Index: patches/patch-sic_c
> ===================================================================
> RCS file: patches/patch-sic_c
> diff -N patches/patch-sic_c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-sic_c 12 Aug 2016 20:01:40 -0000
> @@ -0,0 +1,12 @@
> +$OpenBSD$
> +--- sic.c.orig Fri Aug 12 21:45:07 2016
> ++++ sic.c Fri Aug 12 21:54:39 2016
> +@@ -173,6 +173,8 @@ main(int argc, char *argv[]) {
> + fflush(srv);
> + setbuf(stdout, NULL);
> + setbuf(srv, NULL);
> ++ if (pledge("stdio", NULL) == -1)
> ++ eprint("error: pledge:");
> + for(;;) { /* main loop */
> + FD_ZERO(&rd);
> + FD_SET(0, &rd);
>
>
> --
> jca | PGP: 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Reply | Threaded
Open this post in threaded view
|

Re: [OpenBSD] sic pledge patch

Joerg Jung
In reply to this post by Jeremie Courreges-Anglas-2
On Fri, Aug 12, 2016 at 11:40:12PM +0200, Jeremie Courreges-Anglas wrote:

> "Ali H. Fardan" <[hidden email]> writes:
>
> > On 2016-08-12 23:03, [hidden email] wrote:
> >> If you want to do contributions to the ports tree, remember that many
> >> ports have a maintainer.  This maintainer should either be contacted
> >> first or Cc'ed when sending a patch to ports.
> >
> > apologies
> >
> >>
> >> Probably, but not as is.
> >>
> >
> > of course
> >
> >> This checks has no value for the ports tree, and it would do more harm
> >> than good, should upstream accept the patch as is.  What if pledge(2)
> >> becomes available on another OS?
> >
> > if pledge became available on another OS, it would be their job to use
> > this patch,
>
> From a general POV, if the point of the patch we include in the ports
> tree is to be pushed upstream, I don't see why the use of pledge(2)
> wouldn't be as automatic as possible if available, just like for any
> other function.
>
> > also I wrote the #ifdef because I intended to submit this
> > patch to the mainstream sic, but I changed my mind and I thought that
> > this is the correct place to do it, so it is not necessary to include
> > it.
>
> Joerg knows better than us whether the use of pledge should be pushed
> upstream. :)

I submitted a diff to upstream, let's see how it goes.

Reply | Threaded
Open this post in threaded view
|

Re: [OpenBSD] sic pledge patch

Jeremie Courreges-Anglas-2
In reply to this post by Joerg Jung

Committed, thanks.

--
jca | PGP: 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Reply | Threaded
Open this post in threaded view
|

Re: [OpenBSD] sic pledge patch

Joerg Jung
In reply to this post by Joerg Jung

> On 14 Aug 2016, at 00:33, Joerg Jung <[hidden email]> wrote:
>
> On Fri, Aug 12, 2016 at 11:40:12PM +0200, Jeremie Courreges-Anglas wrote:
>> "Ali H. Fardan" <[hidden email]> writes:
>>
>>> On 2016-08-12 23:03, [hidden email] wrote:
>>>
>>>>
>>>> Probably, but not as is.
>>>>
>>>
>>> of course
>>>
>>>> This checks has no value for the ports tree, and it would do more harm
>>>> than good, should upstream accept the patch as is.  What if pledge(2)
>>>> becomes available on another OS?
>>>
>>> if pledge became available on another OS, it would be their job to use
>>> this patch,
>>
>> From a general POV, if the point of the patch we include in the ports
>> tree is to be pushed upstream, I don't see why the use of pledge(2)
>> wouldn't be as automatic as possible if available, just like for any
>> other function.
>>
>>> also I wrote the #ifdef because I intended to submit this
>>> patch to the mainstream sic, but I changed my mind and I thought that
>>> this is the correct place to do it, so it is not necessary to include
>>> it.
>>
>> Joerg knows better than us whether the use of pledge should be pushed
>> upstream. :)
>
> I submitted a diff to upstream, let's see how it goes.

Finally, upstream committed a variation yesterday.