bugFreeciv - Bugs: bug #21096, "Trying to put -1 into 16...

 
 
Show feedback again

bug #21096: "Trying to put -1 into 16 bits" in send_player_info_c()

Submitted by:  Marko Lindqvist <cazfi>
Submitted on:  Tue 03 Sep 2013 09:10:10 AM UTC  
 
Category: generalSeverity: 3 - Normal
Priority: 5 - NormalStatus: Fixed
Assigned to: Marko Lindqvist <cazfi>Open/Closed: Closed
Release: Operating System: None
Planned Release: 2.4.1, 2.5.0, 2.6.0

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Sun 03 Nov 2013 01:10:12 AM UTC, SVN revision 23682:

Fixed handling of "wonder_city_id" network capability.

Reported by Jacob Nevins

See bug #21096

(Browse SVN revision 23682)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Tue 29 Oct 2013 10:59:37 PM UTC, comment #7:

> if (!has_capability("wonder_city_id", client.conn.capability)) {


Oops. Certainly shouldn't have "!" there.

Fix attached.

As for the consequences of the original bug, I have not looked it closely, but I think I saw at least some code that handled negative values (which WONDER_LOST should be, but was not once transfered to client) different from positive values.

(file #19288)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Tue 29 Oct 2013 10:46:18 PM UTC, comment #6:

For S2_4 client:

Am I totally misreading this, or is the sense of the capability test inverted, such that this always uses the wrong member of packet_player_info?
(Which I think means that it'll always end up 0 == WONDER_NOT_BUILT.)
Also, the first assignment to pplayer->wonders[i] looks redundant (but doesn't save the day).
I was unable to spot any obvious trouble on a quick look with a S2_4 savegame, though.

Apart from the server warning (which isn't on S2_4), I'm guessing that the most likely impact of the original bug -- depending on what happens when the server does htons(-1) -- was that the client thinks any lost wonders are in city ID 65535, which most likely doesn't exist. I can't see many knock-on consequences from that -- in particular, I think city_from_wonder() happens to do the right thing. Might affect some client inferences about what should be grayed out in menus etc? Maybe affects small wonders more (cf multiplayer ruleset)?

Jacob Nevins <jtn>
Project Administrator
Tue 29 Oct 2013 04:58:10 PM UTC, SVN revision 23648:

Send wonder city ID as sint32 to allow special value WONDER_LOST.

See bug #21096

(Browse SVN revision 23648)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Tue 29 Oct 2013 04:57:41 PM UTC, SVN revision 23647:

Send wonder city ID as sint32 to allow special value WONDER_LOST.

See bug #21096

(Browse SVN revision 23647)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Tue 29 Oct 2013 04:57:23 PM UTC, SVN revision 23646:

Send wonder city ID as sint32 to allow special value WONDER_LOST.

See bug #21096

(Browse SVN revision 23646)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sat 26 Oct 2013 09:25:42 PM UTC, comment #2:

Patches. S2_4 version uses optional network capability.

(file #19251, file #19252, file #19253)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sat 26 Oct 2013 08:08:00 PM UTC, comment #1:

Got a reproducible "Trying to put -1 into 16 bits" -error in send_player_info(). The field in question is wonders[<someindex>]. Network protocol expects uint16 city id, but the value packaged can be also WONDER_LOST = -1.

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Tue 03 Sep 2013 09:10:10 AM UTC, original submission:

Every time a player logs in to our multiplayer game there's "Trying to put -1 into 16 bits" warnings. I have not tested if that happens with all (auth enabled) multiplayer games, and cannot much investigate the running production game.

partial backtrace from log:

dio_put_uint16()
...
send_player_info_c()
send_player_info_all()
send_all_info()
...
establish_new_connection()
auth_handle_reply()
server_packet_input()
...

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.

 

(Note: upload size limit is set to 1024 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by jtn (Posted a comment)
  • -unavailable- added by cazfi (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 12 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 03 Nov 2013 01:10:21 AM UTCcazfiOpen/ClosedOpen=>Closed
    Tue 29 Oct 2013 10:59:37 PM UTCcazfiAttached File-=>Added ReversedWonderCapability.patch, #19288
      Open/ClosedClosed=>Open
    Tue 29 Oct 2013 05:01:18 PM UTCcazfiStatusReady For Test=>Fixed
      Assigned toNone=>cazfi
      Open/ClosedOpen=>Closed
    Sat 26 Oct 2013 09:25:42 PM UTCcazfiAttached File-=>Added WonderCitySpecialValue.patch, #19251
      Attached File-=>Added WonderCitySpecialValue-S2_5.patch, #19252
      Attached File-=>Added WonderCitySpecialValue-S2_4.patch, #19253
      StatusNone=>Ready For Test
      Planned Release=>2.4.1, 2.5.0, 2.6.0
    Sat 26 Oct 2013 08:08:00 PM UTCcazfiCategoryNone=>general
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup