Re: Safe C

2014-09-25 Thread Matti Karnaattu
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

2014-09-24 Thread Matti Karnaattu
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

2014-09-24 Thread Matti Karnaattu
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

2014-09-24 Thread Matti Karnaattu
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

2014-09-24 Thread Matti Karnaattu

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

2014-09-24 Thread Matti Karnaattu
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

2014-09-24 Thread Matti Karnaattu
 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

2014-09-24 Thread Matti Karnaattu
 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

2014-09-24 Thread Matti Karnaattu
 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

2014-09-24 Thread Matti Karnaattu
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

2014-09-20 Thread Matti Karnaattu
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

2014-09-20 Thread Matti Karnaattu
Should you add EBADMSG and EPROTO too?



Re: Missing include in sys/ipc.h

2014-09-20 Thread Matti Karnaattu
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

2014-09-20 Thread Matti Karnaattu
 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

2014-09-19 Thread Matti Karnaattu
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

2014-09-18 Thread Matti Karnaattu
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

2014-09-18 Thread Matti Karnaattu
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

2014-09-18 Thread Matti Karnaattu
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

2014-09-18 Thread Matti Karnaattu
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

2014-09-18 Thread Matti Karnaattu
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

2014-09-17 Thread Matti Karnaattu
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

2014-09-16 Thread Matti Karnaattu
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

2014-09-16 Thread Matti Karnaattu
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