svn commit: r268491 - head/usr.bin/users

[Pietro Cerutti]

On 2014-Jul-10, 17:04, Ed Schouten wrote:
> Hi there,
> 
> On 10 July 2014 14:15, Pietro Cerutti <gahr at freebsd.org> wrote:
> >   Reimplements users(1) in C++.
> >
> >   This reduces the lines of code by roughly 50% (not counting the COPYRIGHT
> >   header) and makes it more readable by using standard algorithms.
> 
> Interesting. More C++ in our base system. :-)
> 
> > Modified: head/usr.einnusers/Makefile
> > ==============================================================================
> > --- head/usr.bin/users/Makefile Thu Jul 10 11:20:24 2014        (r268490)
> > +++ head/usr.bin/users/Makefile Thu Jul 10 12:15:02 2014        (r268491)
> > @@ -1,6 +1,7 @@
> >  #      @(#)Makefile    8.1 (Berkeley) 6/6/93
> >  # $FreeBSD$
> >
> > -PROG=  users
> > +WARNS=         3
> > +PROG_CXX=      users
> >
> >  .include <bsd.prog.mk>
> 
> Why does this require WARNS to be set to 3? For brand new code, I
> would have expected that it would just build with WARNS=6.

It's because of the standard library. Examples:

/usr/include/c++/v1/memory:1454:47: error: unused parameter '__a'
[-Werror,-Wunused-parameter]
        static void construct(allocator_type& __a, _Tp* __p)

/usr/include/c++/v1/__functional_base:85:12: error: cast from 'const
volatile char *' to 'wchar_t *' increases required alignment from 1 to 4
[-Werror,-Wcast-align]
    return (_Tp*)&reinterpret_cast<const volatile char&>(__x);

> 
> > +using namespace std;
> 
> Out of curiosity, do we have any style guidelines w.r.t. C++? For
> example, I would personally not use things like "using namespace std".
> It becomes hard to figure out where symbols come from.

I wouldn't use "using namespace" in header files to avoid ns pollution,
but I think it makes the code clearer - by avoiding repeating std:: - in
this case.

> 
> > +       sort(begin(names), end(names));
> > +       vector<string>::iterator last(unique(begin(names), end(names)));
> 
> This could also just be a std::set, right?

Yes, except std::set implements bidirection iterators instead of random
access iterators. Bidirectional iterators do not support arithmetic with
integers, which I need to get to the previous-to-last element in the
very next line of code.

> 
> Even though I actually think C++ is a lot nicer than C, do we really
> think it is actually worth proactively replacing already existent
> tools? This specific tool has now become three times as big as the
> previous version (7128 -> 23840 bytes).

No, I'm not going to systematically replace everything with c++. The
thing is, I had already begun to refactor a bit users(1), and this
looked like the natural next step to me. Of course, here the trade off
between code readability and object size shows.

-- 
Pietro Cerutti
The FreeBSD Project
gahr at FreeBSD.org

PGP Public Key:
http://gahr.ch/pgp
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 949 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/svn-src-all/attachments/20140710/8b1c1896/attachment.sig>