[MPlayer-cvslog] r36904 - trunk/stream/vcd_read_os2.h

[Reimar Döffinger]

On Mon, Feb 24, 2014 at 08:39:44AM +0900, KO Myung-Hun wrote:
> 
> 
> Reimar Döffinger wrote:
> > On 23 February 2014 15:35:31 CET, komh <subversion at mplayerhq.hu> wrote:
> >> Author: komh
> >> Date: Sun Feb 23 15:35:30 2014
> >> New Revision: 36904
> >>
> >> Log:
> >> vcd_read_os2.h: replace calloc() with _calloc() only in vcd_read_os2.h
> > 
> > I meant: why use a define at all?
> 
> I don't want to change the previous codes. And I don't want to use
> #ifdef whenever finding calloc(). Although there is only one calloc()
> this case.
> 
> > I don't think there is any guarantee that it is safe to override calloc via a define at all, plus defines have loads of issues.
> 
> In this case, what problems does define has ? And _lcalloc() should be
> identical to calloc() except that it should allocate a memory in a low
> memory pool not in a any memory pool including a high memory.

Well, theoretically: there might be a
#define calloc __builtin_calloc
in the headers for example, in which case the define will cause warnings
and the undef a behaviour change compared to not having them.
There's also the fact that it makes it less clear where the issue is,
you first have to find the calloc it affects, and then see that it only affects
one.
Then there's the general issues with defines, that if you use a function
define like "#define calloc(a,b) ..." it will not work with function
pointers, whereas a "#define calloc ..." will also affect variables.

> > That is, unless you want to go even further and remove the sector data array completely from the context and just put in on the stack in the read function, then you'd need no ifdef at all (but should add a comment why)
> 
> This was a way of my solutions. But, IIRC, you didn't like the sector
> data array in the stack when reviewing these codes.

Well, it's kind of largish with just above 2kB (I think), and I wasn't
sure how problematic stack size is.

> BTW now you are
> asking to do so. Hmm....

Well, it kind of changes which is uglier/more risky...
It has nice effect that the "hack" is constrained to the
exact place where it is needed.
But anyway, do what you want, it's OS/2 specific code and
it's not me who will maintain it.
It just feels not quite right to me to use a "#define calloc"
for it.