Re: [01/18] windowscodecs: Do not assume that vtable is the first element of the object.
I disagree with many of the choices in this series (for example, I think casting an interface pointer to an inherited interface is fine, and replacing that with QueryInterface just adds unnecessary additional logic), but I don't think any semantics have been changed and overall it's a good cleanup.
Vincent Povirk madewokherd@gmail.com wrote:
I disagree with many of the choices in this series (for example, I think casting an interface pointer to an inherited interface is fine, and replacing that with QueryInterface just adds unnecessary additional logic), but I don't think any semantics have been changed and overall it's a good cleanup.
Please also consider that many of the objects may be implemented by a someone's else codec dll, and relying on an implementation detail is not acceptable in that case.
I disagree with many of the choices in this series (for example, I think casting an interface pointer to an inherited interface is fine, and replacing that with QueryInterface just adds unnecessary additional logic), but I don't think any semantics have been changed and overall it's a good cleanup.
Please also consider that many of the objects may be implemented by a someone's else codec dll, and relying on an implementation detail is not acceptable in that case.
An implementation where the two approaches work differently would be wrong. Every interface with IWICBitmapSource methods should implement them in the same way. In cases where it matters we "should" match native which could be doing either.
Vincent Povirk madewokherd@gmail.com wrote:
I disagree with many of the choices in this series (for example, I think casting an interface pointer to an inherited interface is fine, and replacing that with QueryInterface just adds unnecessary additional logic), but I don't think any semantics have been changed and overall it's a good cleanup.
Please also consider that many of the objects may be implemented by a someone's else codec dll, and relying on an implementation detail is not acceptable in that case.
An implementation where the two approaches work differently would be wrong. Every interface with IWICBitmapSource methods should implement them in the same way. In cases where it matters we "should" match native which could be doing either.
Requesting somebody how he "should" implement things instead of calling QueryInterface() when appropriate and avoiding potentially harmful casts sounds like a not very nice stance. Arguing that native implementation may be doing it wrong doesn't help either.
On 07/24/12 17:47, Dmitry Timoshkov wrote:
Vincent Povirk madewokherd@gmail.com wrote:
I disagree with many of the choices in this series (for example, I think casting an interface pointer to an inherited interface is fine, and replacing that with QueryInterface just adds unnecessary additional logic), but I don't think any semantics have been changed and overall it's a good cleanup.
Please also consider that many of the objects may be implemented by a someone's else codec dll, and relying on an implementation detail is not acceptable in that case.
An implementation where the two approaches work differently would be wrong. Every interface with IWICBitmapSource methods should implement them in the same way. In cases where it matters we "should" match native which could be doing either.
Requesting somebody how he "should" implement things instead of calling QueryInterface() when appropriate and avoiding potentially harmful casts sounds like a not very nice stance. Arguing that native implementation may be doing it wrong doesn't help either.
Casts to inherited interfaces are the kind of casts that are not bad at all. The child interface is perfectly good implementation of parent interface by definition. It's just C that can't reflect it in its typing system. So, while we want to avoid casts as much as possible, we need to live with the fact that we sometimes need to cast inteface pointer to its inherited parent.
BTW, changing them to QueryInterface call is not avoiding any cast. It actually adds more of them in form of cast to void** in caller and assignment to void* in QI implementation (not to mention runtime overhead).
Obviously many other casts you're fixing should indeed be fixed.
Jacek
-
Dmitry Timoshkov -
Jacek Caban -
Vincent Povirk