Unused vtables and debug channels

[Andrew Talbot]

It appears that the following vtables and Wine debug channels are not being used, so I am considering removing them. Please let me know, therefore, if you have plans for any of them and want them kept.


Vtables:

IDirectXFileBinary_Vtbl                 d3dxof/d3dxof.c
IDirectXFileObject_Vtbl                 d3dxof/d3dxof.c
IDirectXFileSaveObject_Vtbl             d3dxof/d3dxof.c
DirectMusicPortDownload_Vtbl            dmusic/portdownload.c
DirectMusicThru_Vtbl                    dmusic/thru.c
DirectMusicSegment8_Segment_Vtbl        dswave/dswave.c
AssemblyCacheItemVtbl                   fusion/asmcache.c
AssemblyEnumVtbl                        fusion/asmcache.c
InkCollectorVtbl                        inkobj/inkcollector.c
HTMLInputTextElementVtbl                mshtml/htmlinput.c
MemInputPin_Vtbl                        quartz/nullrenderer.c
OutputPin_Vtbl*                         quartz/pin.c
InputPin_Vtbl*                          quartz/pin.c
PullPin_Vtbl*                           quartz/pin.c
MemInputPin_Vtbl                        quartz/transform.c
MemInputPin_Vtbl                        quartz/videorenderer.c

*Shadow variable(s) present.


Wine debug channels in:

comctl32/theme_button.c
d3drm/math.c
d3dx8/mesh.c
d3dx9_24/d3dx9_24_main.c
d3dx9_25/d3dx9_25_main.c
d3dx9_26/d3dx9_26_main.c
d3dx9_27/d3dx9_27_main.c
d3dx9_28/d3dx9_28_main.c
d3dx9_30/d3dx9_30_main.c
d3dx9_31/d3dx9_31_main.c
d3dx9_32/d3dx9_32_main.c
d3dx9_33/d3dx9_33_main.c
d3dx9_34/d3dx9_34_main.c
d3dx9_35/d3dx9_35_main.c
d3dx9_36/d3dx9_36_main.c
d3dx9_37/d3dx9_37_main.c
dmime/dmutils.c
dmstyle/dmutils.c
jscript/parser.tab.c
kernel32/fiber.c
kernel32/relay16.c
msvcr71/msvcr71.c
msvcrt40/msvcrt40.c
ole32/oleproxy.c
riched20/table.c
secur32/schannel.c
sxs/sxs.c
winealsa.drv/waveinit.c
winecoreaudio.drv/audiounit.c
winecoreaudio.drv/coremidi.c

Thanks,

-- 
Andy.

[James Hawkins]

On Mon, Dec 15, 2008 at 1:41 PM, Andrew Talbot
<andrew.talbot(a)talbotville.com> wrote:
> It appears that the following vtables and Wine debug channels are not being used, so I am considering removing them. Please let me know, therefore, if you have plans for any of them and want them kept.

Why would you remove any of them?  That's like removing stub functions
because we don't know if they're ever called.


Yes to keeping the following two, and I'm pretty sure you're going to
get the same response for the entire list.  They're stubbed out
because they need to be implemented.  The APIs that provide access to
the objects are stubs as well, which is why you're saying they're not
being used.
> AssemblyCacheItemVtbl                   fusion/asmcache.c
> AssemblyEnumVtbl                        fusion/asmcache.c

-- 
James Hawkins

[Christian Costa]

I Agree.

There was also this can of cleanup in d3dxof and direct music dlls 
whereas there are not complete.
Some code are put in place as placeholder for future developments in one 
hand and there is some clean-up to remove them in the other.
I don't see the point.

Christian

James Hawkins a écrit :
> On Mon, Dec 15, 2008 at 1:41 PM, Andrew Talbot
> <andrew.talbot(a)talbotville.com> wrote:
> > It appears that the following vtables and Wine debug channels are not being used, so I am considering removing them. Please let me know, therefore, if you have plans for any of them and want them kept.
>
> Why would you remove any of them?  That's like removing stub functions
> because we don't know if they're ever called.
>
> Yes to keeping the following two, and I'm pretty sure you're going to
> get the same response for the entire list.  They're stubbed out
> because they need to be implemented.  The APIs that provide access to
> the objects are stubs as well, which is why you're saying they're not
> being used.
> > AssemblyCacheItemVtbl                   fusion/asmcache.c
> > AssemblyEnumVtbl                        fusion/asmcache.c

[Andrew Talbot]

James Hawkins wrote:
> Why would you remove any of them?  That's like removing stub functions
> because we don't know if they're ever called.

OK. I get the message; I shall leave the vtables alone. May I take out the
unused debug channels, though? I presume they can easily be re-introduced
where required.

-- 
Andy.

[Austin English]

On Mon, Dec 15, 2008 at 4:52 PM, Andrew Talbot
<Andrew.Talbot(a)talbotville.com> wrote:
> James Hawkins wrote:
> > Why would you remove any of them?  That's like removing stub functions
> > because we don't know if they're ever called.
>
> OK. I get the message; I shall leave the vtables alone. May I take out the
> unused debug channels, though? I presume they can easily be re-introduced
> where required.
>
> --
> Andy.

The same argument would apply to the debug channels. They don't hurt
anything, and when needed, are easier to use if already in place.

What do you gain by removing them?

-- 
-Austin

[Henri Verbeet]

2008/12/15 Austin English <austinenglish(a)gmail.com>:
> The same argument would apply to the debug channels. They don't hurt
> anything, and when needed, are easier to use if already in place.
>
> What do you gain by removing them?

It doesn't work that way. The question should be if you can justify
keeping a piece of code.

[Henri Verbeet]

2008/12/15 James Hawkins <truiken(a)gmail.com>:
> Why would you remove any of them?  That's like removing stub functions
> because we don't know if they're ever called.

Although it probably doesn't make sense to remove these if people have
plans for them, I do think that in general you shouldn't add code
until it's actually needed.

[James Hawkins]

On Mon, Dec 15, 2008 at 3:36 PM, Henri Verbeet <hverbeet(a)gmail.com> wrote:
> 2008/12/15 James Hawkins <truiken(a)gmail.com>:
> > Why would you remove any of them?  That's like removing stub functions
> > because we don't know if they're ever called.
>
> Although it probably doesn't make sense to remove these if people have
> plans for them, I do think that in general you shouldn't add code
> until it's actually needed.

In general sure, but this is an open source project where time
commitments are rarely guaranteed.  For most of the listed cases, I'm
sure the plan was to implement them more fully as soon as possible.
My point is that code cleanup is good, but not to this extent.
Removing dead code is a worthy goal, but I can't imagine a case where
a stubbed interface implementation is dead code.

-- 
James Hawkins

[Henri Verbeet]

2008/12/15 Andrew Talbot <andrew.talbot(a)talbotville.com>:
> secur32/schannel.c
> This one isn't unused, but all its uses are inside an #ifdef.

[Ken Thomases]

On Dec 15, 2008, at 3:41 PM, Andrew Talbot wrote:
> It appears that the following vtables and Wine debug channels are  
> not being used, so I am considering removing them. Please let me  
> know, therefore, if you have plans for any of them and want them kept.
>
> Wine debug channels in:
>
> [...]
> winecoreaudio.drv/audiounit.c

What debug channel is unused in this file?

-Ken

[Andrew Talbot]

Ken Thomases wrote:
> On Dec 15, 2008, at 3:41 PM, Andrew Talbot wrote:
> > It appears that the following vtables and Wine debug channels are
> > not being used, so I am considering removing them. Please let me
> > know, therefore, if you have plans for any of them and want them kept.
> >
> > Wine debug channels in:
> >
> > [...]
> > winecoreaudio.drv/audiounit.c
>
> What debug channel is unused in this file?
>
> -Ken

Hi Ken,

I used the word "appears" above because this list was suggested by static
analysis. Of course, I would examine each file before actually editing it. I
think this one was flagged because the debug channel is declared before the
bulk of the file is conditionally #ifdef'd in:

WINE_DEFAULT_DEBUG_CHANNEL(wave);

#ifdef HAVE_AUDIOUNIT_AUDIOUNIT_H
...

-- 
Andy.

[Maarten Lankhorst]

Hi Andrew,

Andrew Talbot schreef:
> It appears that the following vtables and Wine debug channels are not being used, so I am considering removing them. Please let me know, therefore, if you have plans for any of them and want them kept.
>
> Vtables:
> ....
> MemInputPin_Vtbl                        quartz/nullrenderer.c
> OutputPin_Vtbl*                         quartz/pin.c
> InputPin_Vtbl*                          quartz/pin.c
> PullPin_Vtbl*                           quartz/pin.c
> MemInputPin_Vtbl                        quartz/transform.c
> MemInputPin_Vtbl                        quartz/videorenderer.c

Feel free to remove the quartz ones, it should be harmless.

Cheers,
Maarten.

[Christian Costa]

Hi Andrew,

BTW, if the vtable are removed, there may be some unused functions. Will they be removed as well ?

Bye,
Christian
> Message du 16/12/08 11:11
> De : "Maarten Lankhorst" 
> A : "Andrew Talbot" 
> Copie à : wine-devel(a)winehq.org
> Objet : Re: Unused vtables and debug channels
>
> Hi Andrew,
>
> Andrew Talbot schreef:
> > It appears that the following vtables and Wine debug channels are not being used, so I am considering removing them. Please let me know, therefore, if you have plans for any of them and want them kept.
> >
> > Vtables:
> > ....
> > MemInputPin_Vtbl                        quartz/nullrenderer.c
> > OutputPin_Vtbl*                         quartz/pin.c
> > InputPin_Vtbl*                          quartz/pin.c
> > PullPin_Vtbl*                           quartz/pin.c
> > MemInputPin_Vtbl                        quartz/transform.c
> > MemInputPin_Vtbl                        quartz/videorenderer.c
>
> Feel free to remove the quartz ones, it should be harmless.
>
> Cheers,
> Maarten.

[Andrew Talbot]

Christian Costa wrote:
> Hi Andrew,
>
> BTW, if the vtable are removed, there may be some unused functions. Will
> they be removed as well ?
>
> Bye,
> Christian

Hi Christian,

Because I was mindful not to remove such things lightly, that is why I
sought feedback from the community, and I shall respect the views expressed
by some, therefore, to leave the vtables alone. However, I do agree with
Henri's view that it is the keeping of pieces of code needs to be justified,
and so am inclined to remove any genuinely unused Wine debug channels, since
they are comparatively simple items and very easy to resurrect if
eventually required.

Thanks to everyone,

-- 
Andy.

[Christian Costa]

Andrew Talbot a écrit :
> Christian Costa wrote:
> > Hi Andrew,
> >
> > BTW, if the vtable are removed, there may be some unused functions. Will
> > they be removed as well ?
> >
> > Bye,
> > Christian
>
> Hi Christian,
>
> Because I was mindful not to remove such things lightly, that is why I
> sought feedback from the community, and I shall respect the views expressed
> by some, therefore, to leave the vtables alone. However, I do agree with
> Henri's view that it is the keeping of pieces of code needs to be justified,
> and so am inclined to remove any genuinely unused Wine debug channels, since
> they are comparatively simple items and very easy to resurrect if
> eventually required.
>
> Thanks to everyone,

Hi Andrew,

Cleaning the code is a good thing. If we take the time to do it the 
right way, that's fine.

Christian

[Christian Costa]

Hi,

I would suggest to keep the ones in pin.c. The file pin.c is a kind of 
"template" collection.

Christian

Maarten Lankhorst a écrit :
> Hi Andrew,
>
> Andrew Talbot schreef:
> > It appears that the following vtables and Wine debug channels are not being used, so I am considering removing them. Please let me know, therefore, if you have plans for any of them and want them kept.
> >
> > Vtables:
> > ....
> > MemInputPin_Vtbl                        quartz/nullrenderer.c
> > OutputPin_Vtbl*                         quartz/pin.c
> > InputPin_Vtbl*                          quartz/pin.c
> > PullPin_Vtbl*                           quartz/pin.c
> > MemInputPin_Vtbl                        quartz/transform.c
> > MemInputPin_Vtbl                        quartz/videorenderer.c
>
> Feel free to remove the quartz ones, it should be harmless.
>
> Cheers,
> Maarten.