Don't use _snprintf for snprintf on Windows! (mtransport)

RESOLVED FIXED

Status

()

Core
WebRTC: Networking
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jesup, Assigned: ted)

Tracking

Trunk
x86_64
Windows 7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Created attachment 652958 [details] [diff] [review]
Patch for snprintf

_snprintf doesn't guarantee null-termination.  Evil.  Totally.

Applies to nrappkit, and also to dtlsidentity.cpp in mtransport.

Patch (not for upstreaming probably) attached.  Needs to be tried on windows.
Attachment #652958 - Flags: review?(ekr)
(Assignee)

Comment 1

5 years ago
I mentioned this on #media, but there is a MSVC CRT API you can use: _snprintf_s. Unfortunately you have to pass _TRUNCATE as one of the parameters to make it work properly, so you can't just #define the problem away. I wrote (without testing) a possible inline snprintf implementation we could use:

int snprintf(char *buffer, size_t n, const char *format, ...)
{
  va_list argp;
  int ret;
  va_start(argp, fmt);
  ret = vsnprintf_s(buffer, n, _TRUNCATE, format, argp);
  va_end(argp);
  return ret;
}
(Assignee)

Updated

5 years ago
Blocks: 699646

Comment 2

5 years ago
Comment on attachment 652958 [details] [diff] [review]
Patch for snprintf

Review of attachment 652958 [details] [diff] [review]:
-----------------------------------------------------------------

::: media/mtransport/third_party/nrappkit/src/port/win32/include/csi_platform.h
@@ +53,5 @@
>  #include <r_types.h>
>  
>  #define strcasecmp _stricmp
>  #define strncasecmp _strnicmp
> +#define snprintf PR_snprintf

I'd rather not pull NSPR in here....

Let's just write a new wrapped version of snprintf() and put it in util/util.c

Do you need me to do that?
(Assignee)

Comment 3

5 years ago
Created attachment 655750 [details] [diff] [review]
don't use _snprintf on Windows in media/mtransport

I fixed up jesup's patch to use the snprintf implementation I pasted above.
Attachment #655750 - Flags: review?(ekr)
(Assignee)

Updated

5 years ago
Attachment #652958 - Attachment is obsolete: true
Attachment #652958 - Flags: review?(ekr)
(Assignee)

Updated

5 years ago
Assignee: ekr → ted.mielczarek
(Assignee)

Comment 4

5 years ago
https://hg.mozilla.org/projects/alder/rev/6b00aa578af2
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Whiteboard: [qa-]

Updated

5 years ago
Attachment #655750 - Flags: review?(ekr)
You need to log in before you can comment on or make changes to this bug.