|
From: <sv...@va...> - 2011-10-28 15:10:40
|
Author: bart
Date: 2011-10-28 16:05:50 +0100 (Fri, 28 Oct 2011)
New Revision: 12249
Log:
Use snprintf() instead of sprintf()
Modified:
trunk/coregrind/m_debuginfo/debuginfo.c
Modified: trunk/coregrind/m_debuginfo/debuginfo.c
===================================================================
--- trunk/coregrind/m_debuginfo/debuginfo.c 2011-10-28 02:53:49 UTC (rev 12248)
+++ trunk/coregrind/m_debuginfo/debuginfo.c 2011-10-28 15:05:50 UTC (rev 12249)
@@ -1100,8 +1100,8 @@
*/
Int mashedSzB = VG_(strlen)(pdbname) + VG_(strlen)(wpfx) + 50/*misc*/;
HChar* mashed = ML_(dinfo_zalloc)("di.debuginfo.dnpdi.1", mashedSzB);
- VG_(sprintf)(mashed, "%s/drive_%c%s",
- wpfx, pdbname[0], &pdbname[2]);
+ VG_(snprintf)(mashed, mashedSzB, "%s/drive_%c%s",
+ wpfx, pdbname[0], &pdbname[2]);
vg_assert(mashed[mashedSzB-1] == 0);
ML_(dinfo_free)(pdbname);
pdbname = mashed;
@@ -1112,8 +1112,8 @@
*/
Int mashedSzB = VG_(strlen)(pdbname) + VG_(strlen)(home) + 50/*misc*/;
HChar* mashed = ML_(dinfo_zalloc)("di.debuginfo.dnpdi.2", mashedSzB);
- VG_(sprintf)(mashed, "%s/.wine/drive_%c%s",
- home, pdbname[0], &pdbname[2]);
+ VG_(snprintf)(mashed, mashedSzB, "%s/.wine/drive_%c%s",
+ home, pdbname[0], &pdbname[2]);
vg_assert(mashed[mashedSzB-1] == 0);
ML_(dinfo_free)(pdbname);
pdbname = mashed;
|
|
From: Florian K. <br...@ac...> - 2011-10-28 20:34:15
|
On 10/28/2011 11:05 AM, sv...@va... wrote:
> Author: bart
> Date: 2011-10-28 16:05:50 +0100 (Fri, 28 Oct 2011)
> New Revision: 12249
>
> Log:
> Use snprintf() instead of sprintf()
>
I'm curious as to why you considered this change.
The allocated buffers are large enough in both cases. So snprintf does
not bring a safety benefit here.
Florian
>
> Modified: trunk/coregrind/m_debuginfo/debuginfo.c
> ===================================================================
> --- trunk/coregrind/m_debuginfo/debuginfo.c 2011-10-28 02:53:49 UTC (rev 12248)
> +++ trunk/coregrind/m_debuginfo/debuginfo.c 2011-10-28 15:05:50 UTC (rev 12249)
> @@ -1100,8 +1100,8 @@
> */
> Int mashedSzB = VG_(strlen)(pdbname) + VG_(strlen)(wpfx) + 50/*misc*/;
> HChar* mashed = ML_(dinfo_zalloc)("di.debuginfo.dnpdi.1", mashedSzB);
> - VG_(sprintf)(mashed, "%s/drive_%c%s",
> - wpfx, pdbname[0], &pdbname[2]);
> + VG_(snprintf)(mashed, mashedSzB, "%s/drive_%c%s",
> + wpfx, pdbname[0], &pdbname[2]);
> vg_assert(mashed[mashedSzB-1] == 0);
> ML_(dinfo_free)(pdbname);
> pdbname = mashed;
> @@ -1112,8 +1112,8 @@
> */
> Int mashedSzB = VG_(strlen)(pdbname) + VG_(strlen)(home) + 50/*misc*/;
> HChar* mashed = ML_(dinfo_zalloc)("di.debuginfo.dnpdi.2", mashedSzB);
> - VG_(sprintf)(mashed, "%s/.wine/drive_%c%s",
> - home, pdbname[0], &pdbname[2]);
> + VG_(snprintf)(mashed, mashedSzB, "%s/.wine/drive_%c%s",
> + home, pdbname[0], &pdbname[2]);
> vg_assert(mashed[mashedSzB-1] == 0);
> ML_(dinfo_free)(pdbname);
> pdbname = mashed;
|
|
From: Bart V. A. <bva...@ac...> - 2011-10-29 09:01:27
|
On Fri, Oct 28, 2011 at 10:34 PM, Florian Krohm <br...@ac...> wrote:
> On 10/28/2011 11:05 AM, sv...@va... wrote:
> > Author: bart
> > Date: 2011-10-28 16:05:50 +0100 (Fri, 28 Oct 2011)
> > New Revision: 12249
> >
> > Log:
> > Use snprintf() instead of sprintf()
> >
> > Modified: trunk/coregrind/m_debuginfo/debuginfo.c
> > ===================================================================
> > --- trunk/coregrind/m_debuginfo/debuginfo.c 2011-10-28 02:53:49 UTC (rev 12248)
> > +++ trunk/coregrind/m_debuginfo/debuginfo.c 2011-10-28 15:05:50 UTC (rev 12249)
> > @@ -1100,8 +1100,8 @@
> > */
> > Int mashedSzB = VG_(strlen)(pdbname) + VG_(strlen)(wpfx) + 50/*misc*/;
> > HChar* mashed = ML_(dinfo_zalloc)("di.debuginfo.dnpdi.1", mashedSzB);
> > - VG_(sprintf)(mashed, "%s/drive_%c%s",
> > - wpfx, pdbname[0], &pdbname[2]);
> > + VG_(snprintf)(mashed, mashedSzB, "%s/drive_%c%s",
> > + wpfx, pdbname[0], &pdbname[2]);
> > vg_assert(mashed[mashedSzB-1] == 0);
> > ML_(dinfo_free)(pdbname);
> > pdbname = mashed;
> > @@ -1112,8 +1112,8 @@
> > */
> > Int mashedSzB = VG_(strlen)(pdbname) + VG_(strlen)(home) + 50/*misc*/;
> > HChar* mashed = ML_(dinfo_zalloc)("di.debuginfo.dnpdi.2", mashedSzB);
> > - VG_(sprintf)(mashed, "%s/.wine/drive_%c%s",
> > - home, pdbname[0], &pdbname[2]);
> > + VG_(snprintf)(mashed, mashedSzB, "%s/.wine/drive_%c%s",
> > + home, pdbname[0], &pdbname[2]);
> > vg_assert(mashed[mashedSzB-1] == 0);
> > ML_(dinfo_free)(pdbname);
> > pdbname = mashed;
>
> I'm curious as to why you considered this change.
> The allocated buffers are large enough in both cases. So snprintf does
> not bring a safety benefit here.
I hadn't even tried to analyze whether the output buffer was large
enough. But I noticed the runtime check after the VG_(sprintf)() call.
That's why I had changed the two VG_(sprintf)() calls into
VG_(snprintf)() calls.
By the way, the code above could be simplified if we had an equivalent
for the asprintf() in the Valgrind source tree.
Bart.
|