Re: Some patches

[Roland Illig <roland illig gmx de>]

Helo Leonard,

Leonard den Ottolander wrote:
> On Tue, 2004-09-28 at 11:42, Roland Illig wrote:
> > The patch for src/slint.c let's me build mc with the option 
> > --with-screen=slang. For any reason the Debian-unstable SLang library 
> > does not contain the symbol _SLsys_input_pending, but it contains 
> > SLang_input_pending, so the patch adjusts the function call.

> A quick glance raises the question whether the test for SLANG_VERSION
> will result in the correct symbols being used on every platform / for
> every version. Debian's slang might not provide that symbol, but other
> slangs might. If this is really Debian specific the main tree might not
> be the proper place to fix your issue. This is where vendor patches are
> for.

I had a look at the SLang code, and SLang_input_pending is just a 
caching wrapper around _SLsys_input_pending. The latter is system 
depended, whereas the former is the function from the official interface.

> However, I wouldn't know if these symbols are interchangeable.

from SLang/UPDATE.txt:
Another change involved the name space.  All external symbols now
start with `SL'.  To this end, the following functions have been
renamed: [...]
--EOF

That might mean that all names that don't start with SL should not be 
used. (Especially if they start with an underscore.)

And they really should be interchangeable. I have run mc with the patch 
and keyboard input works as always.

> And what's the use of the mc_main? Seems just like a wrapper that
> doesn't add anything extra like error handling. And why do you place the
> updated main.c in another patch? These seem to be related and should
> thus be in the same (separate?) patch.

Sorry, that part shouldn't be in the patch right now. For the unit tests 
I am preparing I integrated all mc_SOURCES into a library (libmc.a), 
excluding main(). This way I can include the library and to a black box 
test on all functions that are not declared "static". The "main" 
function is still in the library, but has been renamed to mc_main so 
that my unit test programs can define their own main().

Another benefix from the library generation is that completely unused 
files (currently at least pipethrough.c) will not be included in the mc 
binary.

> > The "named constant" patch replaces literal -1 with INVALID_OFFSET where 
> > appropriate. I am not sure if the one line where INT_MAX is replaced 
> > with INVALID_OFFSET is really correct. Please check.

Well, I'll do.

> By the way, providing a diff -up would make it easier to spot in which
> function the code change is introduced, without having to check the
> code.

Accepted.

> > The patch for src/file.c fixes the prototypes for some functions when mc 
> > is configured --without-largefile.
>
> Same applies as for the introduction of consts before. Check for
> regressions by trying out at least some basic functions that might be
> affected by this code change.

Actually it's just a parameter type change that doesn't affect the code. 
The only problem might be that the functions call one of those 
silently-cast-away-const functions (like strchr), but they don't do. 
They all only do one function call to a function that is itself declared 
to take a const char *.

Roland