This is the mail archive of the cygwin-apps mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch/rebase] Add a rebase database to keep track of DLL addresses


On Jul  6 15:35, Charles Wilson wrote:
> On 7/6/2011 2:30 PM, Corinna Vinschen wrote:
> > On Jul  6 11:27, Charles Wilson wrote:
> >> (And there ARE cases where they CAN be null: e.g. the newly-allocated
> >> entries at the tail of the list -- since you allocate in rounded-up
> >> chunks of 100(?); the name pointers are, of course, explicitly SET to
> >> null when reading data in from the db, prior to reading and allocating
> >> storage for the strings themselves, etc).
> > 
> > Oh, come on.
> 
> I didn't say there was anything wrong.  I'm trying to make sure *I*
> understand: these functions do not check against null, but the
> algorithm, elsewhere, is structured such that null checks are not
> required.  That's fine; I just wanted to be sure I understood.

It didn't read that way.  The comparison functions are used by bsearch
and qsort only, so the argument pointers can never be NULL unless the
array itself is NULL.  The name members are always allocated when a new
member is added to the array, so they can never be NULL, either.  This
was pretty much clear from the start so how do you think does it sound
to get a lecture in a purely theoretical problem?

> Look, I'm used to *extremely* defensive programming on critical embedded
> systems, where a segfault means somebody [might] die. So every function
> has to check and handle illegal input -- or has to document why it
> doesn't need to do so; you don't treat segfaults as a
> programmer-error-detection mechanism ("bug in the calling function").

Point taken.  Nevertheless, even in this scenario you concentrate on
the real problems of a function, not the theoretical ones.

> > Right, but there wouldn't be a problem to do the path conversion before
> > calling ReBaseImage64.
> 
> OK by me.  In fact, we'd probably want to avoid using imagehelper for
> 32bit, but ReBaseImage64 for 64bit; it would make sense to drop
> imagehelper entirely and use ReBaseImage[64] throughout, right?

I think we should either use imagehelper, or the Windows imagehlp lib,
not both.  Whether it's easier to use the Windows lib or to improve
imagehelper is something we have to see.  I'm just starting to look into
the Windows imagehlp lib, and I'm already disappointed to see that all
the functions are not UNICODE capable.  There's also no word about the
used multibyte codeset used to convert the filename to UNICODE.

> > I think this goes over the top.  The patch adds new functionality
> > without breaking the existing one.  Therefore, the new code won't run
> > with additional tweaks on mingw, but it does *not* break mingw.
> 
> Semantics.
> [...]
> Now, in this case we [I] will soon ensure that the feature WILL work, so
> the whiny question will never come up -- but as of the initial,
> cygwin-only introduction...it won't quite work yet.  But the mingw
> executable will /claim/ to have a nifty brand new feature.

So for the time being we could just add #ifdef's which hide the feature
on platforms which are not yet supported.  However, this "doesn't work
on foo" is a simple case of a DLL name on one side and choosing a
filename on the other side.  I just don't care enough for/about the
other targets to make the decisions.  The work itself is probably a 15
minute job for somebody who cares.  This should easily be doable before
the next release.

> > My dictionary doesn't know the word "scrogged", but I guess it's clear from
> > the context :)
> 
> OMG. I just checked the urban dictionary and, um, let's just say that is
> NOT what I meant.  =8-O
> 
> s/scrogged/corrupted/g
> 
> and the less said about that, the better.  I'm going to have to stop
> using that word.

I'll *not* look into the urban dictionary now...

> Dailing things back a little: in this patch you have done all of those
> things you say, and I'm grateful.  My fear was that you were proposing,
> going forward, to STOP doing even minimal accommodations for mingw/msys
> and "concentrate on the Cygwin stuff" because you "don't care for
> mingw".  If that fear was unfounded, then I'm sorry for stating otherwise.
> 
> It may have been a language issue: "don't care for X" implies "do not
> like X", "think poorly of X", and similar -- as in "I don't care for
> boiled beets" -- which puts a very negative light on the "concentrate on
> the Cygwin stuff" statement.  Whereas "don't care *about* X" is probably
> what you meant, and doesn't put such a negative light on the other
> statement.

My dictionary translates "not to care for something" and "not to care
about something" both as "not interested in something" and that's what I
was trying to say.  How this can lead you to fear that I would
deliberately break other targets and especially how you can turn this
into a blunt accusation rather than to *ask* first, beats me.

But, anyway, it's ok now.  I had a night's sleep so I cooled off again.

> I repeat: I have no objections to this patch, as is.  But I'm not the
> maintainer -- Jason is.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]