[Wireshark-dev] Latest libnghttp2 checkin broken

2015-04-30 Thread Joerg Mayer 
The latest checkin to libnghttp2 should not have happend:
a) it breaks compilation on my system
  /home/jmayer/work/wireshark/git/epan/nghttp2/nghttp2_hd.c: In function 
‘hd_inflate_remove_bufs_with_name’:
/home/jmayer/work/wireshark/git/epan/nghttp2/nghttp2_hd.c:1736:10: error: 
variable ‘rv’ set but not used [-Werror=unused-but-set-variable]
   size_t rv;
  ^
and
b) it can only have been committed with --no-verify as it contains deprecated
   functions (checkAPI.pl). I hit this during conflict resolution (git commit).

Either we should remove the lib from the Wireshark source (an inconvenience as
most distros don't provide packages for this) or the code needs to conform to
Wireshark's development environment, i.e. it requires adaption and stricter 
testing
than what seems to be happening right now.

Ciao
   Jörg
-- 
Joerg Mayer   
We are stuck with technology when what we really want is just stuff that
works. Some say that should read Microsoft instead of technology.
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Wireshark runtime messages we don't want to see

2015-04-30 Thread Joerg Mayer 
On Thu, Apr 30, 2015 at 01:44:41PM -0700, Guy Harris wrote:
> On Apr 30, 2015, at 9:58 AM, Joerg Mayer  wrote:
> 
> > jmayer@egg epan$ wireshark
> > ../../asn1/c1222/packet-c1222-template.c:1427:3: runtime error: null 
> > pointer passed as argument 1, which is declared to never be null
> > ../../asn1/c1222/packet-c1222-template.c:1427:3: runtime error: null 
> > pointer passed as argument 2, which is declared to never be null
> 
> That probably means that oid_string2encoded() failed.
> 
> Pascal Quantin's working on a fix for this:
> 
>   https://code.wireshark.org/review/8251
> 
> but presumably the failure means either that
> 
>   1) you specified an invalid (not a syntactically-valid OID) setting for 
> the "Base OID to use for relative OIDs" preference for the C.1222 dissector
> 
> or
> 
>   2) you have no setting for it so that the string is null (which is, as 
> far as I know, not a syntactically-valid OID)
> 
> and I strongly suspect it's 2) here, so perhaps that code should also 
> special-case null strings.  (Ultimately, we should catch 
> syntactically-invalid OIDs rather than just silently ignoring them.)

Yes, it's case 2)

> Hopefully, the code also can handle a missing OID.

 Ciao
   Jörg
-- 
Joerg Mayer   
We are stuck with technology when what we really want is just stuff that
works. Some say that should read Microsoft instead of technology.
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Wireshark runtime messages we don't want to see

2015-04-30 Thread Joerg Mayer 
On Thu, Apr 30, 2015 at 01:08:59PM -0700, Guy Harris wrote:
> On Apr 30, 2015, at 1:00 PM, Roland Knall  wrote:
> 
> > On Thu, Apr 30, 2015 at 6:58 PM, Joerg Mayer  wrote:
> > ERROR: Cannot connect to ADB: Connection refused
> > INFO: Please check that adb daemon is running.
> > 
> > Do not know about the others, but 
> > "ERROR: Cannot connect to ADB: Connection refused
> > INFO: Please check that adb daemon is running."
> > 
> > is the new androiddump extcap utility, and I think it should be ok, that 
> > this message is printed, as it is an external tool, not Wireshark itself.
> 
> Is it run whenever you start Wireshark, even if you don't have any Android 
> development tools installed?

Correct. I don't have any Android development environment installed and it 
appears all
the time.

> If not, is it still run even if you don't happen to be debugging an Android 
> device at the time?
> 
> It should perhaps try to distinguish, if possible, between "failure to 
> connect to ADB when you're actually debugging an Android device" and "failure 
> to connect to ADB because {you're not using any Android development tools 
> right now, you don't even have Android development tools installed, you don't 
> have any Android devices on which to develop, etc.}".

Ciao
 Jörg

-- 
Joerg Mayer   
We are stuck with technology when what we really want is just stuff that
works. Some say that should read Microsoft instead of technology.
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Wireshark runtime messages we don't want to see

2015-04-30 Thread Alexis La Goutte 
On Thu, Apr 30, 2015 at 10:44 PM, Guy Harris  wrote:

>
> On Apr 30, 2015, at 9:58 AM, Joerg Mayer  wrote:
>
> > jmayer@egg epan$ wireshark
> > QMetaObject::connectSlotsByName: No matching signal for
> on_actionExternalMenuItem_triggered()
>
About this warning, i coming from eeed4d1121f122aa5579153872193b9a91d8ad52
We don't use on_xxx_triggered() function, it is a magic Qt function to
automatically "link" signal and slot (
http://qtway.blogspot.no/2010/08/automatic-connections-using-qt-signals.html
)
rename function, it will be fix the warning

> ../../asn1/c1222/packet-c1222-template.c:1427:3: runtime error: null
> pointer passed as argument 1, which is declared to never be null
> > ../../asn1/c1222/packet-c1222-template.c:1427:3: runtime error: null
> pointer passed as argument 2, which is declared to never be null
>
> That probably means that oid_string2encoded() failed.
>
> Pascal Quantin's working on a fix for this:
>
> https://code.wireshark.org/review/8251
>
> but presumably the failure means either that
>
> 1) you specified an invalid (not a syntactically-valid OID)
> setting for the "Base OID to use for relative OIDs" preference for the
> C.1222 dissector
>
> or
>
> 2) you have no setting for it so that the string is null (which
> is, as far as I know, not a syntactically-valid OID)
>
> and I strongly suspect it's 2) here, so perhaps that code should also
> special-case null strings.  (Ultimately, we should catch
> syntactically-invalid OIDs rather than just silently ignoring them.)
>
> Hopefully, the code also can handle a missing OID.
>
> > 18:37:43  Dbg  plugin_dir: /opt/test/lib/wireshark/plugins/1.99.6
> > ERROR: Cannot connect to ADB: Connection refused
> > INFO: Please check that adb daemon is running.
> > ==> after double clicking the wlan0 interface
> > /home/jmayer/work/wireshark/git/wsutil/sign_ext.h:53:33: runtime error:
> left shift of negative value -1
>
> So is this presumably from C99's
>
> 6.5.7 Bitwise shift operators
>
> ...
>
> The result of E1 << E2 is E1 left-shifted E2 bit
> positions; vacated bits are filled with zeros. ... *If E1 has a signed type
> and nonnegative value, and E1 × 2^E2 is representable in the result type,
> then that is the resulting value; otherwise, the behavior is undefined.*
>
> The intent of the shifts in the "val |= (-1 << no_of_bits)" in
>
> /* sign extension routines */
>
> static inline guint32
> ws_sign_ext32(guint32 val, int no_of_bits)
> {
> g_assert (no_of_bits >= 0 && no_of_bits <= 32);
>
> if (no_of_bits == 0)
> return val;
>
> if (val & (1 << (no_of_bits-1)))
> val |= (-1 << no_of_bits);
>
> return val;
> }
>
> static inline guint64
> ws_sign_ext64(guint64 val, int no_of_bits)
> {
> g_assert (no_of_bits >= 0 && no_of_bits <= 64);
>
> if (no_of_bits == 0)
> return val;
>
> if (val & (G_GINT64_CONSTANT(1) << (no_of_bits-1)))
> val |= (G_GUINT64_CONSTANT(-1) << no_of_bits);
>
> return val;
> }
>
> is to generate a value with no_of_bits low-order zero bits and all the
> other bits 1, so that bits in the 32-bit or 64-bit value above the sign bit
> in the no_of_bits-bit value are set for negative values.
>
> (Yes, we're assuming 2's complement, but I don't think this is the only
> place, and if you're trying to get Wireshark to run on a Unisys ClearPath
> Dorado system, trust me, that's not going to be your biggest problem. :-))
>
> We might as well just use (G_GUINT64_CONSTANT(0x) <<
> no_of_bits) - and, while we're at it, use (0x << no_of_bits) in the
> 32-bit case.
>
> Fixed in I62220808058cb93f83329c1916b888a2067d524c.
>
> > /home/jmayer/work/wireshark/git/epan/proto.c:9154:47: runtime error:
> left shift of 1 by 31 places cannot be represented in type 'int'
>
>
> That should now be fixed by I95cf5ce53aa3b94ccb9f246d31863715bb682409, if
> I understand the complaint - bitmasks for testing individual bits should
> probably be given as "1U << xxx" rather than "1 << XXX" so that the type is
> "unsigned int".
>
> There are probably other cases where that needs to be done.  The bit tests
> in the wsutil/sign_ext.h code above is one of them; that's also fixed by
> I62220808058cb93f83329c1916b888a2067d524c.
> ___
> Sent via:Wireshark-dev mailing list 
> Archives:https://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>  mailto:wireshark-dev-requ...@wireshark.org
> ?subject=unsubscribe
>
___
Sent via:Wireshark-dev mailing list 
Archives:https

Re: [Wireshark-dev] Wireshark runtime messages we don't want to see

2015-04-30 Thread Guy Harris 

On Apr 30, 2015, at 9:58 AM, Joerg Mayer  wrote:

> jmayer@egg epan$ wireshark
> QMetaObject::connectSlotsByName: No matching signal for 
> on_actionExternalMenuItem_triggered()
> ../../asn1/c1222/packet-c1222-template.c:1427:3: runtime error: null pointer 
> passed as argument 1, which is declared to never be null
> ../../asn1/c1222/packet-c1222-template.c:1427:3: runtime error: null pointer 
> passed as argument 2, which is declared to never be null

That probably means that oid_string2encoded() failed.

Pascal Quantin's working on a fix for this:

https://code.wireshark.org/review/8251

but presumably the failure means either that

1) you specified an invalid (not a syntactically-valid OID) setting for 
the "Base OID to use for relative OIDs" preference for the C.1222 dissector

or

2) you have no setting for it so that the string is null (which is, as 
far as I know, not a syntactically-valid OID)

and I strongly suspect it's 2) here, so perhaps that code should also 
special-case null strings.  (Ultimately, we should catch syntactically-invalid 
OIDs rather than just silently ignoring them.)

Hopefully, the code also can handle a missing OID.

> 18:37:43  Dbg  plugin_dir: /opt/test/lib/wireshark/plugins/1.99.6
> ERROR: Cannot connect to ADB: Connection refused
> INFO: Please check that adb daemon is running.
> ==> after double clicking the wlan0 interface
> /home/jmayer/work/wireshark/git/wsutil/sign_ext.h:53:33: runtime error: left 
> shift of negative value -1

So is this presumably from C99's

6.5.7 Bitwise shift operators

...

The result of E1 << E2 is E1 left-shifted E2 bit positions; 
vacated bits are filled with zeros. ... *If E1 has a signed type and 
nonnegative value, and E1 × 2^E2 is representable in the result type, then that 
is the resulting value; otherwise, the behavior is undefined.*

The intent of the shifts in the "val |= (-1 << no_of_bits)" in

/* sign extension routines */

static inline guint32
ws_sign_ext32(guint32 val, int no_of_bits)
{
g_assert (no_of_bits >= 0 && no_of_bits <= 32);

if (no_of_bits == 0)
return val;

if (val & (1 << (no_of_bits-1)))
val |= (-1 << no_of_bits);

return val;
}

static inline guint64
ws_sign_ext64(guint64 val, int no_of_bits)
{
g_assert (no_of_bits >= 0 && no_of_bits <= 64);

if (no_of_bits == 0)
return val;

if (val & (G_GINT64_CONSTANT(1) << (no_of_bits-1)))
val |= (G_GUINT64_CONSTANT(-1) << no_of_bits);

return val;
}

is to generate a value with no_of_bits low-order zero bits and all the other 
bits 1, so that bits in the 32-bit or 64-bit value above the sign bit in the 
no_of_bits-bit value are set for negative values.

(Yes, we're assuming 2's complement, but I don't think this is the only place, 
and if you're trying to get Wireshark to run on a Unisys ClearPath Dorado 
system, trust me, that's not going to be your biggest problem. :-))

We might as well just use (G_GUINT64_CONSTANT(0x) << 
no_of_bits) - and, while we're at it, use (0x << no_of_bits) in the 
32-bit case.

Fixed in I62220808058cb93f83329c1916b888a2067d524c.

> /home/jmayer/work/wireshark/git/epan/proto.c:9154:47: runtime error: left 
> shift of 1 by 31 places cannot be represented in type 'int'


That should now be fixed by I95cf5ce53aa3b94ccb9f246d31863715bb682409, if I 
understand the complaint - bitmasks for testing individual bits should probably 
be given as "1U << xxx" rather than "1 << XXX" so that the type is "unsigned 
int".

There are probably other cases where that needs to be done.  The bit tests in 
the wsutil/sign_ext.h code above is one of them; that's also fixed by 
I62220808058cb93f83329c1916b888a2067d524c.
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Wireshark runtime messages we don't want to see

2015-04-30 Thread Roland Knall 
I think, there should be some rules, as to what an extcap interface is
supposed to be able to do and what not. Basically in this case, the device
simply writes to stdout, no matter what.

I do think, that it should not output anything, if called with
--extcap-interfaces.

regards,
Roland

On Thu, Apr 30, 2015 at 10:08 PM, Guy Harris  wrote:

>
> On Apr 30, 2015, at 1:00 PM, Roland Knall  wrote:
>
> > On Thu, Apr 30, 2015 at 6:58 PM, Joerg Mayer  wrote:
> > ERROR: Cannot connect to ADB: Connection refused
> > INFO: Please check that adb daemon is running.
> >
> > Do not know about the others, but
> > "ERROR: Cannot connect to ADB: Connection refused
> > INFO: Please check that adb daemon is running."
> >
> > is the new androiddump extcap utility, and I think it should be ok, that
> this message is printed, as it is an external tool, not Wireshark itself.
>
> Is it run whenever you start Wireshark, even if you don't have any Android
> development tools installed?
>
> If not, is it still run even if you don't happen to be debugging an
> Android device at the time?
>
> It should perhaps try to distinguish, if possible, between "failure to
> connect to ADB when you're actually debugging an Android device" and
> "failure to connect to ADB because {you're not using any Android
> development tools right now, you don't even have Android development tools
> installed, you don't have any Android devices on which to develop, etc.}".
> ___
> Sent via:Wireshark-dev mailing list 
> Archives:https://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>  mailto:wireshark-dev-requ...@wireshark.org
> ?subject=unsubscribe
>
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Wireshark runtime messages we don't want to see

2015-04-30 Thread Guy Harris 

On Apr 30, 2015, at 1:00 PM, Roland Knall  wrote:

> On Thu, Apr 30, 2015 at 6:58 PM, Joerg Mayer  wrote:
> ERROR: Cannot connect to ADB: Connection refused
> INFO: Please check that adb daemon is running.
> 
> Do not know about the others, but 
> "ERROR: Cannot connect to ADB: Connection refused
> INFO: Please check that adb daemon is running."
> 
> is the new androiddump extcap utility, and I think it should be ok, that this 
> message is printed, as it is an external tool, not Wireshark itself.

Is it run whenever you start Wireshark, even if you don't have any Android 
development tools installed?

If not, is it still run even if you don't happen to be debugging an Android 
device at the time?

It should perhaps try to distinguish, if possible, between "failure to connect 
to ADB when you're actually debugging an Android device" and "failure to 
connect to ADB because {you're not using any Android development tools right 
now, you don't even have Android development tools installed, you don't have 
any Android devices on which to develop, etc.}".
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] Wireshark runtime messages we don't want to see

2015-04-30 Thread Roland Knall 
On Thu, Apr 30, 2015 at 6:58 PM, Joerg Mayer  wrote:

> ERROR: Cannot connect to ADB: Connection refused
> INFO: Please check that adb daemon is running.
>

Do not know about the others, but
"ERROR: Cannot connect to ADB: Connection refused
INFO: Please check that adb daemon is running."

is the new androiddump extcap utility, and I think it should be ok, that
this message is printed, as it is an external tool, not Wireshark itself.

regards,
Roland
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

[Wireshark-dev] Wireshark runtime messages we don't want to see

2015-04-30 Thread Joerg Mayer 
jmayer@egg epan$ wireshark
QMetaObject::connectSlotsByName: No matching signal for 
on_actionExternalMenuItem_triggered()
../../asn1/c1222/packet-c1222-template.c:1427:3: runtime error: null pointer 
passed as argument 1, which is declared to never be null
../../asn1/c1222/packet-c1222-template.c:1427:3: runtime error: null pointer 
passed as argument 2, which is declared to never be null
18:37:43  Dbg  plugin_dir: /opt/test/lib/wireshark/plugins/1.99.6
ERROR: Cannot connect to ADB: Connection refused
INFO: Please check that adb daemon is running.
==> after double clicking the wlan0 interface
/home/jmayer/work/wireshark/git/wsutil/sign_ext.h:53:33: runtime error: left 
shift of negative value -1
/home/jmayer/work/wireshark/git/epan/proto.c:9154:47: runtime error: left shift 
of 1 by 31 places cannot be represented in type 'int'

___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] proto_tree_add_text stats

2015-04-30 Thread Jeff Morriss 

Thanks for the good work and massive progress! :-)

On 04/30/15 01:32, Alexis La Goutte wrote:

Thanks for stats !


On Thu, Apr 30, 2015 at 2:44 AM, mailto:mman...@netscape.net>> wrote:

 From the "maybe you were curious, but afraid to ask" file, some
stats I've collected on proto_tree_add_text calls:
1292 calls in the dissector directory [1] from 86 files (for an
average of 15 calls per dissector that have at least one)
21 calls outside of the dissector directory, mostly in comments (all
plugin dissectors have been converted)
There are only 3 dissectors flagged by checkAPIs.pl for "excessive"
proto_tree_add_text use
1. packet-csn1.c (36) .  I've been told this is comparable to asn.1
dissectors.  It seems to me like hf_ items for proto_tree_add_item
should be passed into the functions that call proto_tree_add_text,
but every time I try to figure it out, it just makes my head hurt.
However there are sample captures out there (bug 7615 for
example) if someone wants to take a shot.
2. packet-sigcomp.c (211). This was "hiding" in the epan directory
outside of the dissector directory up until a few months ago.  It
doesn't appear to be 211 separate fields, just a lot of reuse, but I
don't understand enough (or can find sample captures) about it to
take the plunge. convert_proto_tree_add_text.pl
 could probably be used to
lighten the load.  If this dissector is removed from the previous
stats, the average per dissector drops to 12.
3. packet-wbxml.c (80).  This appears to be a tvb_parse candidate,
but I've never seen a sample capture to attempt to make the
conversion (and learn about tvb_parse in the process)
Outside of those 3, according to checkAPIs.pl [2]
1. There are 2 other dissectors that have more than 30
proto_tree_add_text calls
packet-bgp.c (45)
packet-bssgp.c (32)
2. There are 12 dissectors with 20-30 proto_tree_add_text calls
packet-ansi_801.c
packet-ansi_a.c
packet-dcerpc-spoolss.c
packet-dcerpc.c
packet-dvbci.c
packet-gsm_a_rr.c
packet-mih.c
packet-q2931.c
packet-q931.c
packet-rsvp.c
packet-rtps.c
packet-slsk.c
3. There are 21 dissectors with <= 5 proto_tree_add_text calls.
For many of these, I can't match the functionality "exactly" (or how
fields _should_ be formatted) or when "complicated formatting" (or
logic) is involved and I don't have a sample capture to verify against.
As you can tell from the commit logs, I work on this in fits and
starts, but patches are always welcome.  I'll always
defer/discard my work in progress to someone more familiar with a
dissector.
[1] This is a simple grep and doesn't take into account comments,
ignore header files, etc
[2] I've certainly come across a few cases where checkAPIs.pl
results don't equal grep (even if you factor out comments), but
overall its reliable for large numbers of proto_tree_add_text calls


___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe

Re: [Wireshark-dev] proto_tree_add_text stats

2015-04-30 Thread Martin Kaiser 
Thus wrote mman...@netscape.net (mman...@netscape.net):

> 2. There are 12 dissectors with 20-30 proto_tree_add_text calls
> packet-dvbci.c

I'll look into this one, shouldn't be much of a problem.
___
Sent via:Wireshark-dev mailing list 
Archives:https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
 mailto:wireshark-dev-requ...@wireshark.org?subject=unsubscribe