Re: Safe C
Hi Daniel, I'm aware that and dozen others, but I'm interested tooling used or attempts to use in OpenBSD.
chmod.c undefined behavior
I noticed that chmod.c have uninitialized variable char *ep that was used. This diff clarify what I mean. Index: chmod.c === RCS file: /OpenBSD/src/bin/chmod/chmod.c,v retrieving revision 1.30 diff -u -p -u -p -r1.30 chmod.c --- chmod.c 21 May 2014 06:23:01 - 1.30 +++ chmod.c 24 Sep 2014 08:47:58 - @@ -65,7 +65,7 @@ main(int argc, char *argv[]) uid_t uid; gid_t gid; u_int32_t fclear, fset; - char *ep, *mode, *cp, *flags; + char *mode, *cp, *flags; setlocale(LC_ALL, ); @@ -148,13 +148,11 @@ done: flags = *argv; if (*flags = '0' *flags = '7') { errno = 0; - val = strtoul(flags, ep, 8); + val = strtoul(flags, (char *)NULL, 8); if (val UINT_MAX) errno = ERANGE; if (errno) err(1, invalid flags: %s, flags); - if (*ep) - errx(1, invalid flags: %s, flags); fset = val; oct = 1; } else { @@ -167,13 +165,11 @@ done: mode = *argv; if (*mode = '0' *mode = '7') { errno = 0; - val = strtol(mode, ep, 8); + val = strtol(mode, (char *)NULL, 8); if (val INT_MAX || val 0) errno = ERANGE; if (errno) err(1, invalid file mode: %s, mode); - if (*ep) - errx(1, invalid file mode: %s, mode); omode = val; oct = 1; } else {
Re: chmod.c undefined behavior
Now I see it, in the last sentence. I was wrong, it was actually used. I got confused for that monster main function. That is definitely overly complex and that *ep looked like a forgotten. Hmm.. I counted cyclomatic complexity.. and that is 65 in main function :|
Re: chmod.c undefined behavior
That main function is good, standard style. Unnecessary goto, variables defined far away from where they are used, monster function, variables are not commented what they do, not all functions are commented what they do.. To me, it looks like that there is no intention to optimize readability and testability. Instead it looks like there is put effort to minimize amount of functions or something. It has one small getopt(3) loop, decrements argc/argv, has one small codeblock to handle arguments for each variant of the utility, and one concise fts(3) main loop to handle files, and finally a one-liner for error handling. It would be better if they all are put to separate functions. That is the whole point in functions, to put task to single program fragment. So what's the point? You can't *calculate* whether something is readable for a *human*, and that's what matters. There are measured human factors, example size of working memory. If there is measured magic constants, it is possible to calculate what is somewhat better or worse. Something to aim for: -Less linearly independent paths in module -High cohesion -Low coupling These are basics. It also matters a lot for testability. Less independent paths means less test cases.
Re: chmod.c undefined behavior
Or the most straightforward and obvious way to break out of a switch in a loop. True. It is also good way to model exeptions. I take my word back, this may be most elegant way to exit that loop in this case. Variables defined predictably at the start of the function, as the convention is in BSD code. That is good convention. But while the function is so big, it is harder to read. You then also need to shovel data back and forth between these functions. Not necessarily. When the program is this tiny, single file source, it may not so bad thing to use global scope for that shared data. Two hundred lines is hardly long I've used to write less than twenty lines functions. How would you improve these names? Naming is ok me, but I may add comments what they are used for. It sounds like you're just trying to take snippets of code out of context and declare them bad because you're not reading the rest of the code? I don't say that this code is bad. I think it is good gode, but there is plenty of room to improve readability. Much in the same manner that splitting the function into a few dozen would only make it harder to see the code for what it is. And when the lower level details are hidden, it is easier to see what the code does. This is idea of abstraction. Mind you, the code isn't written for a first-year comp sci course where the students need annotated twenty-line snippets to help them come to grips with the basic syntax and structure of a computer program. I like to write self documenting code. My practice is to comment ~all functions with block comments, all parameters (with ranges), return values, exceptions, external effects, all variable declarations (with ranges). And, I prefer to write tests first. Again trying to please some quality metric? Yes, cyclomatic complexity. If that is high, it is usually good sign that code needs refactoring. Did you intend to achieve better cohesion and lower coupling by breaking the function into a dozen small functions that appease whatever metric you're using? No, lower CC index per function instead. This kind of refactoring often reveals opportunities to increase cohesion in more complex programs and helps to reduce the amount of code. Of course, I'm a nobody to say how this code should look, but it looks fine to me; and so far as I can tell, it's not broken. Don't fix it if it's not broken. Yes, it is considerably harder to refactor because there is no tests for that. I think that should be starting point. But if your comments about the current code are any indication of what your ideal version would look like, I'm not sure there are many who would be willing to commit it. I think that it is not defined enough unambiguously, how ideal code looks like. It reduces motivation to improve code better, if it is not defined, what is better. style(9) is good, but it would be better if there is reference component that is optimized to perfection, including tests, naming conventions, file hierarchy etc. Then ALL code changes and new code can be compared to that reference, and that reference code can be used to build templates automated testing.
Re: chmod.c undefined behavior
Bringing that up is the only way to find out what OpenBSD developers do consider poor quality, what they don't, and what is merely considered a matter of style here Thanks for your patience. I feel like I'm querying preferred coding style by this way. I also think that better way to do that is that it would be good thing if there is some reference component in source tree, and rest of the code should be aimed to same quality. In testsuite, style and implementation. I don't think that the goto is problem, even there may be possibility to make clean, readable exit using while comparison. That monster size main function is the issue and reduces readability. mostly a matter of style, not of code quality; I think these are somewhat bonded. Especially in C, because the language is kind of unsecured gun like some Javascript. While the language lacks certain things like classes, exceptions, coroutines etc. this gap should be compensate by improving the coding conventions and checking of those can be automated too. Obviously, you are honestly hunting for bugs. I have about million lines of build and other errors under review where I have found anomalies. Some of them seems to be errors in OpenBSD, most of them are errors in code. It just not work if I point out every single anomaly, bug, missing parameter etc. Example, I think it would be best if I write proper document where I collect ALL known issues related to POSIX conformance that can be reviewed and made to public knowledge.
Re: chmod.c undefined behavior
It's not as easy as that. Not only is the code constantly improved, our standards which idioms are considered good and which are considered dangerous constantly evolve as well, slowly turning new, high quality code into old code of lower quality - even if the code remains completely unchanged. Actually, the quality of code diminishes in time *in particular* when it remains unchanged. I understand. And, the code itself is ultimately the specification. And that reason, why not select single component to be as a reference? And the reference component: This is finest peace of code at the moment, Make rest of the code looks like this, Everything that is diffent to reference is wrong way to do, If some convention just doesn't work, fixed it to reference first The best way to learn is to read code, understand it, find weaknesses, suggest improvements, and when we agree on the improvements, systematically audit the tree for similar problems. Some of the improvements may require enormous amount of effort to apply whole tree, and requires also that when applying changes, have to continuously learn/keep in own knowledge different source components. I don't expect that everyone have interest to all of the components, so it doesn't motivate to hack all components. That must be taken into account Like you say, quality standards are rising all the time, I think it is good to have reference component and accept, that rest of the source tree may have little behind in quality. I have used this practice in my own code. Every time I learn new tool/language/framework etc. I make first component to reference and then apply all practices rest of the components. I've never tried this on larger scale, in open source projects.
Re: chmod.c undefined behavior
If you try to write down a long list, that is not ideal. You will waste a lot of time, and others may not even find the time to read the whole list. True. As OpenBSD aims for POSIX compatibility, I'm thinking about document of missing features, known anomalies, known bugs and reasons for those. Example, if some certain standard feature is simply crap, or there is legacy reasons why things works differently or there is known bugs or some features are not just implemented, these can be checked from single document, there are possible workarounds, OpenBSD specific #ifdef / #endif etc. After I fully review my self all those millions error messages, end result is less than 50 relevant issues. And after that data is reviewed together, @tech there may be left less than 20. And if issues are fixed, new features done, list can be cleaned up, or if new problems are found these can be added. 2014-09-24 21:54 GMT+03:00 Ingo Schwarze schwa...@usta.de: Hi Matti, Matti Karnaattu wrote on Wed, Sep 24, 2014 at 09:14:45PM +0300: Thanks for your patience. I feel like I'm querying preferred coding style by this way. That's not the *goal*, but it's an unavoidable side effect, and the more it succeeds, the easier everuthing gets for both sides. [...] I have about million lines of build and other errors under review where I have found anomalies. Some of them seems to be errors in OpenBSD, most of them are errors in code. It is likely that a sizeable fraction is just false positives, too; but i don't doubt some things hide in there that merit improvement. It just not work if I point out every single anomaly, bug, missing parameter etc. Example, I think it would be best if I write proper document where I collect ALL known issues related to POSIX conformance that can be reviewed and made to public knowledge. Not really. Usually, the main effort when approaching a list of hundreds or thousands of potential issues is - to figure out what is most relevant - to figure out whether it is really an issue - to figure out how exactly it can be improved If you try to write down a long list, that is not ideal. You will waste a lot of time, and others may not even find the time to read the whole list. So really try to pick - out of your huge pile of potential issues - some of those that you consider among the most serious and at the same time the most easy to fix and start with those. Once you found something that is confirmed to be an actual issue and a fix was committed, sweep the tree for it, then move on to the next class of issues. There is no way anyone can fix the world. Everybody has to pick something. Picking something relevant one is able to fix is part of the art - actually, a very important part of the art. Yours, Ingo
Re: chmod.c undefined behavior
You were probably trained in the Object-oriented (and likely a bit of post-OO) tradition. Most likely you learned Java, and then possibly either C# or Ruby. I started C-64 Basic, then I learned assembler, then C. After C, I have learned bunch object oriented and functional languages. Lately I have programmed in Typescript. I actually switch language to another all the time. Language is a tool and similar patterns can be implemented everywhere. OO is not the only valid way to program. I agree. Usually, I'm thinking functional approach first, and if that doesn't cut, I look problem from dataflow perspective and after that I try OO. That usually solve the problem if everything else fails but that it is not of course only way. I don't always agree with the OpenBSD developers (and sometimes I vehemently disagree with their take on things), but as a user I'm much happier with the quality, consistency and predictability that OpenBSD provides, than with some project that's always chasing the moving target of the perfect programming paradigm. I also like how directory structure is done in that scale and that consistency that ~everything is C and sh scripts. and so perhaps more familiar to many modern programmers, but not intrinsically or automatically more readable. I don't afraid of long functions. I also write long functions if I have fully implemented DbC or there is just lot to do. However, I do avoid complex functions. and it's highly unlikely that any random group of programmers will be willing to follow the style you were trained to think is best. best is not best if it is not possible to measure it any practical way. I can measure speed, I can measure memory usage, I can even measure how much implementation uses energy (which is actually good optimum for speed/memory usage) but I agree that coding style is often a matter of opinion. However, there is some factors that actually can be measured and those tells something about quality. Example, -Code coverage in tests (complex code is not easy to test!) -Cyclomatic complexity -Coupling Relating to coding style, it can't practically measure what is better name for function. Despite for that, I consider naming convention extemely important, because that is part of the process that is not so straightforward but I don't ever start argue how to shorten names or is camelCase better. I don't care as long as the style doesn't change. OpenBSD defines (AFAIK) results to include safety of code, and/or freedom from unanticipated side-effects (i.e. bugs). I'm fascinated many ways the clean source tree in that scale and I have personal itch to implement methods to find bugs. Instead of pointing out single bug, I'm thinking about implementing methods to make much as possible bugs in code visible.
Re: chmod.c undefined behavior
But you didn't write any tests before this diff, and as a result you broke chmod. Retrofitting test suite requires some effort and didn't do that now because... My apologies for two things: 1. I do, forgot to put question mark. My intention was to _ask_ for that code: WTF this? 2. I put that message to wrong mailing list. Questions with diff should definitely go to misc@ And now this thread sprawled long way to off course in wrong list... So, I don't answer in this thread any longer, and I hope we all can now shut down this spamming. Even I could easily refactor code to more sane in terms of function complexity, I don't want do that without test suite (there is probably available) because I don't want to take risk of breaking it. And, I don't do that even if I have that test suite, because I have doubt no one will not commit refactored version because there is opinions that monster function can be more readable. Nothing more to see here.
Missing include in sys/ipc.h
Hello, I managed to get build error which was caused a missing include. test: === #include sys/shm.h int main(void) { return 0; } === And here is diff that fixes the bug: Index: ipc.h === RCS file: /OpenBSD/src/sys/sys/ipc.h,v retrieving revision 1.12 diff -u -p -u -p -r1.12 ipc.h --- ipc.h 10 Jul 2014 14:16:49 - 1.12 +++ ipc.h 20 Sep 2014 10:05:01 - @@ -48,6 +48,8 @@ #ifndef _SYS_IPC_H_ #define _SYS_IPC_H_ +#include sys/types.h + struct ipc_perm { uid_t cuid; /* creator user id */ gid_t cgid; /* creator group id */
Re: Missing EOVERFLOW
Should you add EBADMSG and EPROTO too?
Re: Missing include in sys/ipc.h
Oh, looks like man pages should be fixed too. There is no mention in POSIX 2008 that sys/types.h and sys/ipc.h should be included: http://pubs.opengroup.org/onlinepubs/9699919799/functions/shmctl.html http://pubs.opengroup.org/onlinepubs/9699919799/functions/shmget.html http://pubs.opengroup.org/onlinepubs/9699919799/functions/shmat.html So this means that current behavior is not exactly what POSIX 2008 says.
Re: Missing include in sys/ipc.h
Unfortunately it doesn't allow us to make everything in sys/types.h available though. So simply including sys/types.h from sys/ipc.h isn't the right solution. Good point, I didn't notice that, I was just testing sys/shm.h based on what POSIX 2008 specification says and detected that this isn't working as it should.
Missing EOVERFLOW
Hello, intro(2) manual says, also http://pubs.opengroup.org/onlinepubs/9699919799/ that EOVERFLOW should be defined. -snip- #define _XOPEN_SOURCE 700 #include sys/mman.h #include errno.h int main() { errno = EACCES; errno = EAGAIN; errno = EBADF; errno = EINVAL; errno = EMFILE; errno = ENODEV; errno = ENOMEM; errno = ENOTSUP; errno = ENXIO; errno = EOVERFLOW; } -snip- I found that EOVERFLOW is actually found from headers under _BSD_VISIBLE, but this may indicate bug from mmap() Is this known issue? Btw is there bugzilla or some other bug tracking system where I can check my findings?
Re: Fix for POSIX conformance issue
No, it just shouldn't be present when a conforming environment is requested. I've fixed unistd.h. to exclude it when you ask for an XSI conforming environment by building with _XOPEN_SOURCE defined. (Not sure what about signal.h you're referring to there...) http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/signal.h.html sigignore, sigset, SIG_POLL etc. There is plenty of obsolete stuff that is missing from OpenBSD. I assume that missing of obsolete stuff is intentional. Can you clarify why setpgrp should work otherwise? -Matti
Re: Fix for POSIX conformance issue
I also noticed that these uses too setpgrp: binutils/gdb sendmail perl But these are imported so changing may make maintenance harder. However, these uses setpgrp: /usr.bin/sudo/logging.c /usr.sbin/amd/amd/amd.c: /usr.sbin/mrouted/main.c ..but looks like compiler skips it so it may be better to look these better so there may be possibility to remove #ifdef #endif Here is better diff: This changes only code that should fix setpgrp-setpgid. That proc.c looks similar in many files, so there may be possibility to do some refactoring. Index: sbin/iked/proc.c === RCS file: /OpenBSD/src/sbin/iked/proc.c,v retrieving revision 1.19 diff -u -p -u -p -r1.19 proc.c --- sbin/iked/proc.c18 Aug 2014 09:43:02 - 1.19 +++ sbin/iked/proc.c18 Sep 2014 21:24:11 - @@ -352,7 +352,7 @@ proc_run(struct privsep *ps, struct priv fatal(proc_run: cannot fork); case 0: /* Set the process group of the current process */ - setpgrp(0, getpid()); + setpgid(0, getpid()); break; default: return (pid); Index: usr.sbin/lpr/lpd/printjob.c === RCS file: /OpenBSD/src/usr.sbin/lpr/lpd/printjob.c,v retrieving revision 1.52 diff -u -p -u -p -r1.52 printjob.c --- usr.sbin/lpr/lpd/printjob.c 7 Feb 2014 23:06:21 - 1.52 +++ usr.sbin/lpr/lpd/printjob.c 18 Sep 2014 21:24:19 - @@ -153,7 +153,7 @@ printjob(void) (void)close(fd); } pid = getpid(); /* for use with lprm */ - setpgrp(0, pid); + setpgid(0, pid); /* we add SIGINT to the mask so abortpr() doesn't kill itself */ memset(sa, 0, sizeof(sa)); Index: usr.sbin/rarpd/rarpd.c === RCS file: /OpenBSD/src/usr.sbin/rarpd/rarpd.c,v retrieving revision 1.53 diff -u -p -u -p -r1.53 rarpd.c --- usr.sbin/rarpd/rarpd.c 6 Apr 2012 18:03:52 - 1.53 +++ usr.sbin/rarpd/rarpd.c 18 Sep 2014 21:24:19 - @@ -170,7 +170,7 @@ main(int argc, char *argv[]) (void) close(f); } (void) chdir(/); - (void) setpgrp(0, getpid()); + (void) setpgid(0, getpid()); devnull = open(_PATH_DEVNULL, O_RDWR); if (devnull = 0) { (void) dup2(devnull, STDIN_FILENO); Index: usr.sbin/relayd/proc.c === RCS file: /OpenBSD/src/usr.sbin/relayd/proc.c,v retrieving revision 1.16 diff -u -p -u -p -r1.16 proc.c --- usr.sbin/relayd/proc.c 18 Aug 2014 12:59:00 - 1.16 +++ usr.sbin/relayd/proc.c 18 Sep 2014 21:24:19 - @@ -354,7 +354,7 @@ proc_run(struct privsep *ps, struct priv fatal(proc_run: cannot fork); case 0: /* Set the process group of the current process */ - setpgrp(0, getpid()); + setpgid(0, getpid()); break; default: return (pid); Index: usr.sbin/snmpd/proc.c === RCS file: /OpenBSD/src/usr.sbin/snmpd/proc.c,v retrieving revision 1.11 diff -u -p -u -p -r1.11 proc.c --- usr.sbin/snmpd/proc.c 18 Aug 2014 13:13:42 - 1.11 +++ usr.sbin/snmpd/proc.c 18 Sep 2014 21:24:20 - @@ -352,7 +352,7 @@ proc_run(struct privsep *ps, struct priv fatal(proc_run: cannot fork); case 0: /* Set the process group of the current process */ - setpgrp(0, getpid()); + setpgid(0, getpid()); break; default: return (pid);
pthread bug
This doesn't compile: #include pthread.h pthread_once_t once_control; int main() { pthread_t new_thread; once_control = PTHREAD_ONCE_INIT; return 1; }
Re: pthread bug
Oh, I forgot to mention that I'm running current and I have this error few days ago too. man/pthread_once.3 says that this is ok but looks like something is missing from headers. -Matti 2014-09-19 1:33 GMT+03:00 Matti Karnaattu mkarnaa...@gmail.com: This doesn't compile: #include pthread.h pthread_once_t once_control; int main() { pthread_t new_thread; once_control = PTHREAD_ONCE_INIT; return 1; }
Re: pthread bug
It's broken code: PTHREAD_ONCE_INIT is only required to be an initializer, not a value. Ok, thanks.
Re: Fix for POSIX conformance issue
As far as POSIX goes, setpgrp is spelled setpgid. Let's just replaces the instances of setpgrp() in the tree with setpgid() and be done with it. I just noted, that it was marked obsolescent in latest POSIX version. To make it unified with signal.h interface, it should be removed.from unistd.h. -Matti
Fix for POSIX conformance issue
Hi, I found that OpenBSD setpgrp is not POSIX compliant, so I write test and make diff to fix issue. http://pubs.opengroup.org/onlinepubs/009695399/functions/setpgrp.html This change removes obsolete setpgrp, and fix /usr/src where it is used. I can compile userland and it works. Work to do: 1. This is my first diff and I don't know process how to fix man page, can someone advice me? 2. grep -R /usr/src/setpgrp shows that old way is used a few times but skipped by preprocessor directives. can someone advice me with source tree and practices? As some of the code is imported from elsewhere and cleaning those are probably not good idea if we are not forking here. Cleaning process itself is very straightforward. 3. This change break backward compatibility to old BSD code favor to POSIX, so it would be good to inform that fix: setpgrp(pid, pid) - setpgid(pid, pid) NEW FILE: regress/lib/libc/setpgrp === /* * Copyright (c) 2014 Matti Karnaattu mkarnaa...@gmail.com * * Permission to use, copy, modify, and distribute this software for any * purpose with or without fee is hereby granted, provided that the above * copyright notice and this permission notice appear in all copies. * * THE SOFTWARE IS PROVIDED AS IS AND THE AUTHOR DISCLAIMS ALL WARRANTIES * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ #include unistd.h /* * POSIX.1 have System V style setpgrp calling convention that differs * what traditionally have been in BSD systems. POSIX.1 setpgrp takes * no arguments and puts the calling process in its on group like * setpgid(0, 0). */ int main(void) { return (int)(setpgrp() != setpgid(0, 0)); } === Index: include/unistd.h === RCS file: /OpenBSD/src/include/unistd.h,v retrieving revision 1.92 diff -u -p -u -p -r1.92 unistd.h --- include/unistd.h1 Sep 2014 05:09:52 - 1.92 +++ include/unistd.h16 Sep 2014 21:27:31 - @@ -427,7 +427,6 @@ int nice(int); ssize_t readlink(const char * __restrict, char * __restrict, size_t) __attribute__ ((__bounded__(__string__,2,3))); int setkey(const char *); -int setpgrp(pid_t pid, pid_t pgrp);/* obsoleted by setpgid() */ int setregid(gid_t, gid_t); int setreuid(uid_t, uid_t); voidswab(const void *, void *, size_t); @@ -464,6 +463,7 @@ void*sbrk(int); #if __POSIX_VISIBLE = 200112 || __XPG_VISIBLE = 420 int lockf(int, int, off_t); +int setpgrp(void); #endif #if __POSIX_VISIBLE = 200112 || __XPG_VISIBLE = 420 || __BSD_VISIBLE Index: lib/libc/compat-43/setpgrp.c === RCS file: /OpenBSD/src/lib/libc/compat-43/setpgrp.c,v retrieving revision 1.6 diff -u -p -u -p -r1.6 setpgrp.c --- lib/libc/compat-43/setpgrp.c8 Aug 2005 08:05:33 - 1.6 +++ lib/libc/compat-43/setpgrp.c16 Sep 2014 21:27:32 - @@ -32,7 +32,7 @@ #include unistd.h int -setpgrp(pid_t pid, pid_t pgid) +setpgrp(void) { - return(setpgid(pid, pgid)); + return(setpgid(0, 0)); } Index: regress/lib/libc/Makefile === RCS file: /OpenBSD/src/regress/lib/libc/Makefile,v retrieving revision 1.43 diff -u -p -u -p -r1.43 Makefile --- regress/lib/libc/Makefile 3 Jul 2014 21:12:24 - 1.43 +++ regress/lib/libc/Makefile 16 Sep 2014 21:27:35 - @@ -5,7 +5,7 @@ SUBDIR+= atexit basename cephes cxa-atex SUBDIR+= explicit_bzero fmemopen fnmatch fpclassify getcap getopt_long glob SUBDIR+= hsearch longjmp locale malloc mkstemp modf netdb open_memstream SUBDIR+= orientation popen printf -SUBDIR+= regex setjmp setjmp-signal sigreturn sigsetjmp sprintf +SUBDIR+= regex setjmp setjmp-signal setpgrp sigreturn sigsetjmp sprintf SUBDIR+= stdio_threading stpncpy strerror strtod strtol strtonum SUBDIR+= telldir time timingsafe vis Index: sbin/iked/proc.c === RCS file: /OpenBSD/src/sbin/iked/proc.c,v retrieving revision 1.19 diff -u -p -u -p -r1.19 proc.c --- sbin/iked/proc.c18 Aug 2014 09:43:02 - 1.19 +++ sbin/iked/proc.c16 Sep 2014 21:27:38 - @@ -352,7 +352,7 @@ proc_run(struct privsep *ps, struct priv fatal(proc_run: cannot fork); case 0: /* Set the process group of the current process */ - setpgrp(0, getpid
Re: Fix for POSIX conformance issue
All right! I'm not yet convenient with contributing open source projects with source changes, and I don't know yet what is the most efficient way to put effort. I assume that every part of the source tree has some developer responsibility who knows it best and is the most effective working on it, so I try to find what is best way to contribute. Example of this, as you say it is best to use first setpgid and then fix setpgrp and it is best that everyone check own part of code. I'm talking here about to get focus what to do. I'm moderate to spot bugs, security holes, and other issues so I'm thinking process that is that OK, if I write tests that poke OpenBSD to crash, or make before-clean-code to smell like sh*t, and try to NOT patch issue if it is scattered everywhere in the source tree like this. And if the issue is found single component I try to fix it. Writing tests means easily full load of new files, so what is preferred way to contribute? I also noted that regress folder structure may possible need refactoring, and the coding style is very variable there. -Matti