On Sun, Sep 23, 2018 at 12:47:34AM +0200, Klemens Nanni wrote: > If the port you're working on has a maintainer, please include them in > To: or Cc:. I just Cc'ed Kaashif.
Thank you, I tagged the wrong person on this. > > On Sat, Sep 22, 2018 at 02:44:37PM -0400, Jake Champlin wrote: > > >From mentions on bsd.network: > > - Fixes pledge call to follow convention > > - Adds REVISION to Makefile > > - Adds `# uses pledge()` comment to Makefile > These seem fine however your submission lacks important details: > > What did you test and how? I tested the every codepath's possible execution points by running the game with every possible runtime option, including (-r) which resets the highscore file and exits. > > Which promises are needed for what? stdio - Needed for curses functions, as well as highscore writes. rpath - Needed for `access` in highscore.c. wpath - Needed to write the highscore file, and reset the highscore file. cpath - Needed to create the highscore file, if it doesn't exist. tty - Needed for curses functions. > > Looking at it's usage and manual, I see no options to control behaviour > towards the highscore file. Does that mean it's always written? > If not, this would indicate code paths where it can run with less > promises. Yes, unfortunately, the only code path where the highscore file is not read is on help output, which occurs in src/options.c. > > In general, please explain your diffs so people don't have guess and/or > redo all the work. Good point, sorry about that. > > > COMMENT = terminal version of the 2048 sliding block puzzle game > > +REVISION = 0 > REVISION usually goes below the version information; GH_TAGNAME in this > case. This should also be a tab after "=". Fixed. > > > Index: patches/patch-pledge > > =================================================================== > > RCS file: patches/patch-pledge > Patches are named after the patched source file as done by > `update-patches'. See bsd.port.mk(5) as well as our porting guide in > our FAQ. Awesome, thanks for the information, updated as well. > > > diff -N patches/patch-pledge > > --- /dev/null 1 Jan 1970 00:00:00 -0000 > > +++ patches/patch-pledge 22 Sep 2018 18:44:16 -0000 > > @@ -0,0 +1,26 @@ > Missing CVS tag. > > > +Pledges 2048 > No need for such an obvious comment imho. Agreed, removed. Thanks for the extra comments on this, and for looking over the changes! Index: Makefile =================================================================== RCS file: /cvs/ports/games/2048-cli/Makefile,v retrieving revision 1.1.1.1 diff -u -p -u -p -r1.1.1.1 Makefile --- Makefile 10 Dec 2017 22:44:51 -0000 1.1.1.1 +++ Makefile 23 Sep 2018 01:00:07 -0000 @@ -6,12 +6,14 @@ CATEGORIES = games GH_ACCOUNT = tiehuis GH_PROJECT = 2048-cli GH_TAGNAME = v0.9.1 +REVISION = 0 MAINTAINER = Kaashif Hymabaccus <[email protected]> # MIT PERMIT_PACKAGE_CDROM = Yes +# uses pledge() WANTLIB += c curses USE_GMAKE = Yes Index: patches/patch-src_main_c =================================================================== RCS file: patches/patch-src_main_c diff -N patches/patch-src_main_c --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ patches/patch-src_main_c 23 Sep 2018 01:00:07 -0000 @@ -0,0 +1,35 @@ +$OpenBSD$ + +Adds pledge() call to 2048. + +Promises needed: +stdio - Needed for curses and writing highscore file. +rpath - Needed for access in highscore.C +wpath - Needed for writes to highscore file, and for resetting the file. +cpath - Needed to create the highscore file, if it doesn't exist. +tty - Needed for curses functions. + +This was tested against multiple code paths, with verified results. + +Index: src/main.c +--- src/main.c.orig ++++ src/main.c +@@ -1,5 +1,6 @@ + #include <stdio.h> + #include <stdbool.h> ++#include <unistd.h> + #include "ai.h" + #include "engine.h" + #include "gfx.h" +@@ -13,6 +14,11 @@ void draw_then_sleep(struct gfx_state *s, struct games + + int main(int argc, char **argv) + { ++ if (pledge("stdio rpath wpath cpath tty", NULL) == -1) { ++ perror("pledge"); ++ return 1; ++ } ++ + struct gamestate *g = gamestate_init(argc, argv); + struct gfx_state *s = NULL; +
