Bruce, your comments do make sense. I semi-seriously suggest that we get rid of the current implementation and replace it with this. Comments?
/*
* Copyright (c) 2014 Pietro Cerutti <g...@freebsd.org>
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
* 4. Neither the name of the University nor the names of its contributors
* may be used to endorse or promote products derived from this software
* without specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
* ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
* OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE.
*/
#include <sys/cdefs.h>
__FBSDID("$FreeBSD: head/usr.bin/users/users.c 267027 2014-06-03 20:59:26Z gahr
$");
#include <utmpx.h>
#include <algorithm>
#include <cstdlib>
#include <iostream>
#include <iterator>
#include <string>
#include <vector>
using namespace std;
int
main(int argc, char **)
{
struct utmpx *ut;
vector<string> names;
if (argc > 1) {
cerr << "usage: users" << endl;
return (1);
}
setutxent();
while ((ut = getutxent()) != NULL) {
if (ut->ut_type != USER_PROCESS)
continue;
names.push_back(ut->ut_user);
}
endutxent();
if (names.size() == 0) {
return (0);
}
sort(begin(names), end(names));
vector<string>::iterator last(unique(begin(names), end(names)));
copy(begin(names), last-1, ostream_iterator<string>(cout, " "));
cout << *(last-1) << endl;
}
On 2014-Jun-05, 02:20, Bruce Evans wrote:
> On Tue, 3 Jun 2014, Pietro Cerutti wrote:
>
> > Log:
> > - Avoid calling a wrapper function around strcmp
>
> This changes correct code to give undefined behaviour.
>
> > - Use sizeof(*array) instead of sizeof(element) everywhere
>
> This also allows removal of a typedef obfuscation.
>
> > Modified: head/usr.bin/users/users.c
> > ==============================================================================
> > --- head/usr.bin/users/users.c Tue Jun 3 20:58:11 2014
> > (r267026)
> > +++ head/usr.bin/users/users.c Tue Jun 3 20:59:26 2014
> > (r267027)
> > @@ -50,9 +50,9 @@ static const char rcsid[] =
> > #include <unistd.h>
> > #include <utmpx.h>
> >
> > -typedef char namebuf[sizeof(((struct utmpx *)0)->ut_user) + 1];
> > +typedef char namebuf[sizeof(((struct utmpx *)0)->ut_user) + 1];
> > +typedef int (*scmp)(const void *, const void *);
>
> Most typedefs shouldn't exist, and these 2 are no exceptions.
>
> 'namebuf' only existed to reduce 2 copies of a type declaration to 1.
> It obfuscated both. Now it is only used once.
>
> 'scmp' only exists to help implement undefined behaviour.
>
> The change also de-KNFizes the formatting (from tab to space after the
> first part of the type).
>
> > @@ -91,7 +91,7 @@ main(int argc, char **argv)
> > }
> > endutxent();
> > if (ncnt > 0) {
> > - qsort(names, ncnt, sizeof(namebuf), scmp);
> > + qsort(names, ncnt, sizeof(*names), (scmp)strcmp);
>
> qsort()'s function pointer must point to a function like scmp, not
> a different type of function with the type mismatch hidden by a bogus
> cast.
>
> The type puns fail as follow:
> - strcmp is initially a function
> - dereferencing it gives a pointer to a function of the type of strcmp
> - casting this pointer to (scmp) gives a pointer to a different type
> of function. The behaviour is defined. The representation may
> change.
> - use of such a cast function pointer to call the function is undefined
> unless the pointer is converted back to the original (pointer) type
> before making the call. Since qsort() cannot possibly know the original
> type, it cannot convert back.
>
> It is a feature that qsort()'s API doesn't have a function pointer typedef.
> There are a few legitimate uses for function pointer typedefs, but in
> practices most uses are for bogus casts to hide undefined behaviour, as
> above.
>
> The qsort() call also uses the other type obfuscation. 'namebuf' is
> the typedef-name. This typedef name was only used twice, first to
> declare 'names' and also here. Now with the better spelling of the
> sizeof(), the typedef name is only used once. It just obfuscates
> the declaration of 'names'.
>
> > ...
> > @@ -107,10 +107,3 @@ usage(void)
> > fprintf(stderr, "usage: users\n");
> > exit(1);
> > }
> > -
> > -int
> > -scmp(const void *p, const void *q)
> > -{
> > -
> > - return (strcmp(p, q));
> > -}
>
> Most qsort() functions have to look like this, or a bit more complicated,
> to avoid the undefined behaviour. This one looks simpler than it is.
> It has to convert the arg types to those of strcmp(), but this happens
> automatically due to strcmp()s prototype. The arg types started as
> strings, but had to be converted to const void *'s to match qsort()'s
> API, then have to be converted back to strings here.
>
> The undefined behaviour is usually to work in practice because only
> Vax hardware is supported. strings are so similar to void * that it
> is hard to tell if the behaviour can ever be undefined in practice
> for type puns between them. But the behaviour is more clearly undefined
> for function pointers because compatibility of void * and char * as
> args doesn't extend to the function pointers. The implementation could
> reasonably check all types at runtime and is only constrained from
> generating a fatal error for type mismatches for certain mismatches
> that are specified to work for compatibility reasons. I think that
> if the bogus conversions get as far as calling the function, then
> the behaviour is defined by the compatibility kludges:
> - string args became const void * in qsort()'s internals
> - they are not converted back when they are passed to strcmp()
> - however, the compatibility kludges now apply. const void * looks
> enough like const char * to work. (I think it may have a different
> representation, but only with bits that aren't really used, for
> example extra type bits that might be used for error checking
> but must not be used here due to the compatibility kludges.)
>
> Bruce
--
Pietro Cerutti
The FreeBSD Project
g...@freebsd.org
PGP Public Key:
http://gahr.ch/pgp
pgpwCbsbF0mpd.pgp
Description: PGP signature
