Re: CVS commit: src

[christos%zoulas.com@localhost (Christos Zoulas)]

On Jan 30,  1:34am, tsutsui%ceres.dti.ne.jp@localhost (Izumi Tsutsui) wrote:
-- Subject: Re: CVS commit: src

| > I know all that, but you are not answering the question. What is broken
| > right now and what are you trying to fix? There is no struct winsize
| > anymore in roken.h...
| 
| Hmm.
| 
|  * Isn't it better to have no changes against src/crypto/dist other
|    than including nbtools_config.h just for dumb two host tools?
|  * It is not a good idea to edit a generated and expoterd file, either.
|    You removed asnprintf() and vasnprintf() from roken.h
|    but not roken.h.in, for example.
|  * src/tools/compat/configure doesn't check all header files
|    referred in src/include/heimdal/roken.h, and the roken.h
|    doesn't have HAVE_FOO_H checks.
|    Some OS might still fail in asn1_compile and compile_et.
|  * There might also be more function decls that don't match for
|    ones in some OS like asnprintf().
|  * get_windows_size() is an exported function in /usr/lib/libroken.so
|    and src/include/heimdal/roken.h is also installed as <krb5/roken.h>.
|    You might have to bump major if you changed an ABI, strictly.
| 
| If you don't think these could be called broken,
| I'm just a paranoia and will shut up.

1. We use nbtools_config.h to provide portability compiling netbsd
   programs on other platforms. For things like gcc we use the native
   config system. The two heimdal programs look more like netbsd applications
   than standalone config candidates.
2. Whoever generated roken.h from roken.h.in, hand edited it afterwards
   anyway. The autoconfiguration found that we did not have asnprintf
   and vasnprintf and included those functions in roken.h. We never
   added sprintf.c in the Makefile for libroken so that was just wrong.
   Removing them from roken.h.in is not a change that will be accepted
   upstream.
3. For the most part roken.h includes (or should include) posix headers.
   I would like to move into that direction instead. Right now it includes
   the kitchensink and it does not need to.
4. Could be. I don't like to code solutions for future/non existing
   problems though.
5. Yes, perhaps changing the api was a mistake from the perspective
   that kerberos programs might expect the old API. It was a stupid
   API decision in the first place, but what can we do. I have not
   found anything that breaks from that change yet, that is why I
   have not reverted it, have you? I still can revert the change
   and declare the struct conditionally:

#ifndef TIOCGWINSZ
        struct winsize {
        ...
        }
#endif

   and avoid using autoconf. I just thought it was silly to depend on
   the actual non-portable API to get the number of lines and columns
   portably :-)


christos