On Thu, 2 Aug 2012, Samuel Pitoiset wrote:

---
libavformat/rtmpproto.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c
index 1341cd7..c130b2b 100644
--- a/libavformat/rtmpproto.c
+++ b/libavformat/rtmpproto.c
@@ -1033,7 +1033,11 @@ static int handle_invoke(URLContext *s, RTMPPacket *pkt)
    const uint8_t *data_end = pkt->data + pkt->data_size;
    int ret;

-    //TODO: check for the messages sent for wrong state?
+    if (pkt->data[0] != AMF_DATA_TYPE_STRING) {
+        av_log(s, AV_LOG_ERROR, "No string method found in invoke packet\n");
+        return AVERROR_INVALIDDATA;
+    }
+
    if (!memcmp(pkt->data, "\002\000\006_error", 9)) {
        uint8_t tmpstr[256];

Is this necessary? As far as I can see, each of the memcmps below also check that the first byte is 2, so this check doesn't change anything at all, except printing a clearer warning if this would happen (I doubt it ever happens in practice except when dealing with broken servers, and I'm not sure if we should litter our code base with extra debugging messages just to aid someone fixing their broken implementations).

If it isn't necessary, as I see it, but just is a convenience/extra warning (or cleanup for future code), it would be much easier to accept the change if you say so in the commit message. Now it took me time to try to figure out why it was needed.

Also, this removes a todo message about something totally different - this check doesn't do anything remotely related to that todo.

Also, I've asked it before and I'll ask it again - please please send separate patches in a patch series in the same mail thread (send them all in the same git send-email invocation, or if you send them separately, set the in-reply-to header correctly). Right now there's a 1/2 patch thread at one place and a 2/2 patch thread elsewhere, and it's very hard afterwards to figure out whether these two belong together, or which one is the missing 1/2 for this one. If they're in the same mail thread, things become much simpler.

// Martin
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to