Re: [RFC] removing broken includes

On Sat Feb  5 11, Giorgos Keramidas wrote:
> On Fri, 4 Feb 2011 20:19:48 +0000, Alexander Best <arundel_at_freebsd.org> wrote:
> >On Fri Feb  4 11, John Baldwin wrote:
> >>On Thursday, February 03, 2011 5:12:57 pm Alexander Best wrote:
> >>> hi everybody,
> >>> i've started to check the source for broken includes, such as the one fixed in
> >>> r218189. so far i've run through arch=amd64 and found only two broken includes,
> >>> which should be fixed by the attached patch. the commands i'm using are:
> >>
> >> The sys/conf/files fix should definitely go in.
> >
> > as a side note: while checking the tinderbox output i saw a few gas related
> > warnings, which complained about VCS IDs being spammed to object files.
> 
> Maybe it's intentional for these objects to _have_ the VCS id in them,
> like some of the "_at_(#)" strings that are passed down to the ELF binary.
> 
> This sort of object data is often used to determine what bits have gone
> into the binary, so we should check if they are intentional before
> ifdef'ing them all out.
> 
> % Index: sbin/routed/rtquery/rtquery.c
> % ===================================================================
> % --- sbin/routed/rtquery/rtquery.c       (revision 218217)
> % +++ sbin/routed/rtquery/rtquery.c       (working copy)
> % _at__at_ -49,6 +49,7 _at__at_
> %  #include <bstring.h>
> %  #endif
> %
> % +#if 0
> %  #define UNUSED __attribute__((unused))
> %  #ifndef __RCSID
> %  #define __RCSID(_s) static const char rcsid[] UNUSED = _s
> % _at__at_ -67,6 +68,7 _at__at_
> %  __RCSID("$Revision: 2.26 $");
> %  #ident "$Revision: 2.26 $"
> %  #endif
> % +#endif
> 
> We can avoid this by building rtquery.c with -D NO__RCSID.  See below
> for the same trick about lukemftpd...
> 
> %  #ifndef sgi
> %  #define _HAVE_SIN_LEN
> % Index: contrib/lukemftp/src/main.c
> % ===================================================================
> % --- contrib/lukemftp/src/main.c (revision 218217)
> % +++ contrib/lukemftp/src/main.c (working copy)
> % _at__at_ -96,8 +96,10 _at__at_
> %
> %  #include <sys/cdefs.h>
> %  #ifndef lint
> % +#if 0
> %  __COPYRIGHT("_at_(#) Copyright (c) 1985, 1989, 1993, 1994\n\
> %         The Regents of the University of California.  All rights reserved.\n");
> % +#endif
> %  #endif /* not lint */
> 
> This looks 'harmless' to include in the resulting binary, but maybe we
> should just define NO_COPYRIGHT while building lukemftp?  This will
> disable the spamming of the object code with the static string and we
> don't have to modify vendor code at all.
> 
> So, how about this instead?

i've compared the VCS info in main.c in the freebsd repository with the newest
vendor release:

FreeBSD:
-------

#include <sys/cdefs.h>
#ifndef lint
__COPYRIGHT("_at_(#) Copyright (c) 1985, 1989, 1993, 1994\n\
        The Regents of the University of California.  All rights reserved.\n");
#endif /* not lint */

#ifndef lint
#if 0
static char sccsid[] = "_at_(#)main.c      8.6 (Berkeley) 10/9/94";
#else
__RCSID("$NetBSD: main.c,v 1.94 2005/05/13 05:03:49 lukem Exp $");
#endif
#endif /* not lint */

VENDOR:
------

#if 0   /* tnftp */
 
#include <sys/cdefs.h>
#ifndef lint
__COPYRIGHT("_at_(#) Copyright (c) 1985, 1989, 1993, 1994\
 The Regents of the University of California.  All rights reserved.\
  Copyright 1996-2008 The NetBSD Foundation, Inc.  All rights reserved");
#endif /* not lint */
 
#ifndef lint
#if 0
static char sccsid[] = "_at_(#)main.c      8.6 (Berkeley) 10/9/94";
#else
__RCSID(" NetBSD: main.c,v 1.117 2009/07/13 19:05:41 roy Exp  ");
#endif
#endif /* not lint */
 
/*
 * FTP User Program -- Command Interface.
 */
#include <sys/types.h>
#include <sys/socket.h>
 
#include <err.h>
#include <errno.h>
#include <netdb.h>
#include <paths.h>
#include <pwd.h>
#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
#include <unistd.h>
#include <locale.h>

#endif  /* tnftp */

... so it seems having the cvs data in object files isn't essential. however
leaving the source files for lukemftp and rtquery the way they are and adding
the -D options in order to prevent object spamming is fine for me too. :)

> 
> % diff -r bd0305f6cb2b libexec/lukemftpd/Makefile
> % --- a/libexec/lukemftpd/Makefile        Sat Feb 05 02:39:55 2011 +0100
> % +++ b/libexec/lukemftpd/Makefile        Sat Feb 05 22:28:12 2011 +0100
> % _at__at_ -13,6 +13,7 _at__at_ SRCS= cmds.c conf.c ftpd.c ftpcmd.y pope
> %  SRCS+= strsuftoll.c
> %
> %  WFORMAT= 0
> % +CFLAGS+= -DNO_COPYRIGHT
> %
> %  .if ${MK_INET6_SUPPORT} != "no"
> %  CFLAGS+= -DINET6
> 
> > also i noticed a lot of these warnings:
> >
> > usr/subversion-src/lib/libc/i386/string/memcpy.S:7: Warning: ignoring incorrect section type for .note.GNU-stack
> >
> > i think i've seen some recent commits dealing with that new section.
> 
> That looks like something more useful to fix now, rather than later.

...already fixed by kib in r218303 and r218305.

cheers.
alex

-- 
a13x