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 Sep 3 09:10:10 2013  
 
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.0Contains string changes: None

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 Nov 3 01:10:12 2013, 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 Oct 29 22:59:37 2013, 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 Oct 29 22:46:18 2013, 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 Oct 29 16:58:10 2013, 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 Oct 29 16:57:41 2013, 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 Oct 29 16:57:23 2013, 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 Oct 26 21:25:42 2013, 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 Oct 26 20:08:00 2013, 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 Sep 3 09:10:10 2013, 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.

     

    Error: not logged in

     

     

    Follow 12 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun Nov 3 01:10:21 2013cazfiOpen/ClosedOpen=>Closed
    Tue Oct 29 22:59:37 2013cazfiAttached File-=>Added ReversedWonderCapability.patch, #19288
      Open/ClosedClosed=>Open
    Tue Oct 29 17:01:18 2013cazfiStatusReady For Test=>Fixed
      Assigned toNone=>cazfi
      Open/ClosedOpen=>Closed
    Sat Oct 26 21:25:42 2013cazfiAttached 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 Oct 26 20:08:00 2013cazfiCategoryNone=>general
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup