[prev in list] [next in list] [prev in thread] [next in thread]
List: wireshark-dev
Subject: [Wireshark-dev] Optimizations (tvbuff)
From: Anders Broman <anders.broman () ericsson ! com>
Date: 2013-10-16 15:36:40
Message-ID: 43C5658BA3FB7B48A6F38EED0B6253F11A6B64B0 () ESESSMB105 ! ericsson ! se
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
Hi,
I just noted that tvb_get_string is quite expensive looking at the code I c=
an see that it calls tvb_memcpy both routines have length check
Which seems unnecessary I've included a patch for review. Feel free to appl=
y if it looks OK. There may be similar cases if someone cares to take a loo=
k :)
Regards
Anders
[Attachment #5 (text/html)]
<html xmlns:v="urn:schemas-microsoft-com:vml" \
xmlns:o="urn:schemas-microsoft-com:office:office" \
xmlns:w="urn:schemas-microsoft-com:office:word" \
xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" \
xmlns="http://www.w3.org/TR/REC-html40"> <head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<meta name="Generator" content="Microsoft Word 14 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:Wingdings;
panose-1:5 0 0 0 0 0 0 0 0 0;}
@font-face
{font-family:Wingdings;
panose-1:5 0 0 0 0 0 0 0 0 0;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0cm;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri","sans-serif";}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
span.EmailStyle17
{mso-style-type:personal-compose;
font-family:"Calibri","sans-serif";
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri","sans-serif";}
@page WordSection1
{size:612.0pt 792.0pt;
margin:72.0pt 72.0pt 72.0pt 72.0pt;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal">Hi,<o:p></o:p></p>
<p class="MsoNormal">I just noted that tvb_get_string is quite expensive looking at \
the code I can see that it calls tvb_memcpy both routines have length \
check<o:p></o:p></p> <p class="MsoNormal">Which seems unnecessary I’ve included \
a patch for review. Feel free to apply if it looks OK. There may be similar cases if \
someone cares to take a look <span \
style="font-family:Wingdings">J</span><o:p></o:p></p> <p \
class="MsoNormal">Regards<o:p></o:p></p> <p class="MsoNormal">Anders <o:p></o:p></p>
</div>
</body>
</html>
["tvbuff.patch" (application/octet-stream)]
Index: epan/tvbuff.c
===================================================================
--- epan/tvbuff.c (revision 2231)
+++ epan/tvbuff.c (working copy)
@@ -709,6 +709,23 @@
/************** ACCESSORS **************/
+inline static void *
+_tvb_memcpy(tvbuff_t *tvb, void *target, guint abs_offset, guint abs_length)
+{
+ if (tvb->real_data) {
+ return memcpy(target, tvb->real_data + abs_offset, abs_length);
+ }
+
+ if (tvb->ops->tvb_memcpy)
+ return tvb->ops->tvb_memcpy(tvb, target, abs_offset, abs_length);
+
+ /* XXX, fallback to slower method */
+
+ DISSECTOR_ASSERT_NOT_REACHED();
+ return NULL;
+
+}
+
void *
tvb_memcpy(tvbuff_t *tvb, void *target, const gint offset, size_t length)
{
@@ -730,17 +747,7 @@
DISSECTOR_ASSERT(length <= 0x7FFFFFFF);
check_offset_length(tvb, offset, (gint) length, &abs_offset, &abs_length);
- if (tvb->real_data) {
- return memcpy(target, tvb->real_data + abs_offset, abs_length);
- }
-
- if (tvb->ops->tvb_memcpy)
- return tvb->ops->tvb_memcpy(tvb, target, abs_offset, abs_length);
-
- /* XXX, fallback to slower method */
-
- DISSECTOR_ASSERT_NOT_REACHED();
- return NULL;
+ return _tvb_memcpy(tvb, target, abs_offset, abs_length);
}
@@ -770,7 +777,7 @@
check_offset_length(tvb, offset, (gint) length, &abs_offset, &abs_length);
duped = wmem_alloc(scope, abs_length);
- return tvb_memcpy(tvb, duped, abs_offset, abs_length);
+ return _tvb_memcpy(tvb, duped, abs_offset, abs_length);
}
@@ -1209,7 +1216,7 @@
guid->data1 = tvb_get_ntohl(tvb, offset);
guid->data2 = tvb_get_ntohs(tvb, offset + 4);
guid->data3 = tvb_get_ntohs(tvb, offset + 6);
- tvb_memcpy(tvb, guid->data4, offset + 8, sizeof guid->data4);
+ _tvb_memcpy(tvb, guid->data4, offset + 8, sizeof guid->data4);
}
void
@@ -1219,7 +1226,7 @@
guid->data1 = tvb_get_letohl(tvb, offset);
guid->data2 = tvb_get_letohs(tvb, offset + 4);
guid->data3 = tvb_get_letohs(tvb, offset + 6);
- tvb_memcpy(tvb, guid->data4, offset + 8, sizeof guid->data4);
+ _tvb_memcpy(tvb, guid->data4, offset + 8, sizeof guid->data4);
}
/*
@@ -1914,7 +1921,7 @@
tvb_ensure_bytes_exist(tvb, offset, length); /* make sure length = -1 fails */
strbuf = (guint8 *)wmem_alloc(scope, length + 1);
- tvb_memcpy(tvb, strbuf, offset, length);
+ _tvb_memcpy(tvb, strbuf, offset, length);
strbuf[length] = '\0';
return strbuf;
}
___________________________________________________________________________
Sent via: Wireshark-dev mailing list <wireshark-dev@wireshark.org>
Archives: http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
mailto:wireshark-dev-request@wireshark.org?subject=unsubscribe
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic