Signed-off-by: Patrick Hibbs hibbsncc1701@gmail.com --- dlls/msdmo/dmoreg.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/dlls/msdmo/dmoreg.c b/dlls/msdmo/dmoreg.c index 8e0680931f4..42e4eab3651 100644 --- a/dlls/msdmo/dmoreg.c +++ b/dlls/msdmo/dmoreg.c @@ -451,10 +451,16 @@ static BOOL any_types_match(const DMO_PARTIAL_MEDIATYPE *a, unsigned int a_count
for (i = 0; i < a_count; ++i) { - for (j = 0; j < b_count; ++j) + if (a != NULL) { - if (IsMediaTypeEqual(&a[i], &b[j])) - return TRUE; + for (j = 0; j < b_count; ++j) + { + if (b != NULL) + { + if (IsMediaTypeEqual(&a[i], &b[j])) + return TRUE; + } + } } } return FALSE;
Hello Patrick,
On 9/30/20 8:43 AM, Patrick Hibbs wrote:
Signed-off-by: Patrick Hibbs hibbsncc1701@gmail.com
dlls/msdmo/dmoreg.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/dlls/msdmo/dmoreg.c b/dlls/msdmo/dmoreg.c index 8e0680931f4..42e4eab3651 100644 --- a/dlls/msdmo/dmoreg.c +++ b/dlls/msdmo/dmoreg.c @@ -451,10 +451,16 @@ static BOOL any_types_match(const DMO_PARTIAL_MEDIATYPE *a, unsigned int a_count
for (i = 0; i < a_count; ++i) {
for (j = 0; j < b_count; ++j)
if (a != NULL) {
if (IsMediaTypeEqual(&a[i], &b[j]))return TRUE;
for (j = 0; j < b_count; ++j){if (b != NULL){if (IsMediaTypeEqual(&a[i], &b[j]))return TRUE;}} }
This seems like the wrong solution; "types" shouldn't be NULL if "size" is nonzero. That it is is a bug in itself, which I diagnosed in [1] but never got around to sending a patch for...
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256
On Wed, 2020-09-30 at 10:55 -0500, Zebediah Figura wrote:
Hello Patrick,
On 9/30/20 8:43 AM, Patrick Hibbs wrote:
Signed-off-by: Patrick Hibbs hibbsncc1701@gmail.com
dlls/msdmo/dmoreg.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/dlls/msdmo/dmoreg.c b/dlls/msdmo/dmoreg.c index 8e0680931f4..42e4eab3651 100644 --- a/dlls/msdmo/dmoreg.c +++ b/dlls/msdmo/dmoreg.c @@ -451,10 +451,16 @@ static BOOL any_types_match(const DMO_PARTIAL_MEDIATYPE *a, unsigned int a_count
for (i = 0; i < a_count; ++i) {
for (j = 0; j < b_count; ++j)
if (a != NULL) {
if (IsMediaTypeEqual(&a[i], &b[j]))return TRUE;
for (j = 0; j < b_count; ++j){if (b != NULL){if (IsMediaTypeEqual(&a[i], &b[j]))return TRUE;}} }This seems like the wrong solution; "types" shouldn't be NULL if "size" is nonzero. That it is is a bug in itself, which I diagnosed in [1] but never got around to sending a patch for...
Hello Zebediah,
Yeah, that's the same bug.
However, I still think the patch should be looked at. (Although I did just notice a bit of redundancy on my part. That check should only be done once per function call.)
Wine will crash whatever thread winds up calling this function if the a and / or b argument is set to NULL. If that's not supposed to happen when "size" > 0, then it should be checked for and enforced by the code before any derefs occur.
As the deref occurs in this function it makes sense to check for and enforce the expectations here.
As it stands, wine crashes because it violated it's own assumptions. In my case, fixing the issue by enforcing the assumption allowed another type to deal with the media stream. Which allowed the program I was debugging to work as intended. I think that would be a better potential outcome for wine's users than wine crashing abruptly.
What do you think?
- -- Patrick
On 9/30/20 11:58 AM, Patrick Hibbs wrote:
On Wed, 2020-09-30 at 10:55 -0500, Zebediah Figura wrote:
Hello Patrick,
On 9/30/20 8:43 AM, Patrick Hibbs wrote:
Signed-off-by: Patrick Hibbs hibbsncc1701@gmail.com
dlls/msdmo/dmoreg.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/dlls/msdmo/dmoreg.c b/dlls/msdmo/dmoreg.c index 8e0680931f4..42e4eab3651 100644 --- a/dlls/msdmo/dmoreg.c +++ b/dlls/msdmo/dmoreg.c @@ -451,10 +451,16 @@ static BOOL any_types_match(const DMO_PARTIAL_MEDIATYPE *a, unsigned int a_count
for (i = 0; i < a_count; ++i) {
for (j = 0; j < b_count; ++j)
if (a != NULL) {
if (IsMediaTypeEqual(&a[i], &b[j]))return TRUE;
for (j = 0; j < b_count; ++j){if (b != NULL){if (IsMediaTypeEqual(&a[i], &b[j]))return TRUE;}} }This seems like the wrong solution; "types" shouldn't be NULL if "size" is nonzero. That it is is a bug in itself, which I diagnosed in [1] but never got around to sending a patch for...
Hello Zebediah,
Yeah, that's the same bug.
However, I still think the patch should be looked at. (Although I did just notice a bit of redundancy on my part. That check should only be done once per function call.)
Wine will crash whatever thread winds up calling this function if the a and / or b argument is set to NULL. If that's not supposed to happen when "size" > 0, then it should be checked for and enforced by the code before any derefs occur.
As the deref occurs in this function it makes sense to check for and enforce the expectations here.
As it stands, wine crashes because it violated it's own assumptions. In my case, fixing the issue by enforcing the assumption allowed another type to deal with the media stream. Which allowed the program I was debugging to work as intended. I think that would be a better potential outcome for wine's users than wine crashing abruptly.
What do you think?
It's a tradeoff, I guess, between zealously assert()ing every time something should be true, and never doing so, and I think in this case the assert just wouldn't be worthwhile. It's a common enough pattern that some_array should contain valid space for at least some_array_count elements, that I think the code is pretty self-explanatory as it is, and I get the impression usual Wine code agrees with that level of robustness.
-- Patrick
ἔρρωσο, Zebediah
-
Patrick Hibbs -
Zebediah Figura