[Freeciv-Dev] [bug #15498] crash in remove_trade_route() - fc_assert_action()?
URL:
http://gna.org/bugs/?15498
Summary: crash in remove_trade_route() - fc_assert_action()?
Project: Freeciv
Submitted by: syntron
Submitted on: Samstag 27.02.2010 um 13:54
Category: general
Severity: 3 - Normal
Priority: 5 - Normal
Status: None
Assigned to: None
Originator Email:
Open/Closed: Open
Release: 2.2.0
Discussion Lock: Any
Operating System: None
Planned Release:
___
Details:
using trunk (svn 16958) I get the following error:
1: in remove_trade_route() [citytools.c::1995]: assertion 'pc1 pc2'
failed.
As 'pother_city' is NULL in the backtrace, it seems to be that the 'action'
('continue' here) is not executed in fc_assert_action().
for (o = 0; o NUM_TRADE_ROUTES; o++) {
struct city *pother_city = game_find_city_by_number(pcity-trade[o]);
fc_assert_action(pcity-trade[o] == 0 || pother_city != NULL, continue);
remove_trade_route(pother_city, pcity);
}
The backtrace is
#2 0x7faa7ac28329 in __assert_fail () from /lib64/libc.so.6
No symbol table info available.
#3 0x0054a61e in fc_assert_fail (file=value optimized out,
function=value optimized out,
line=value optimized out, assertion=value optimized out,
message=value optimized out) at log.c:436
__PRETTY_FUNCTION__ = fc_assert_fail
#4 0x0044e5eb in remove_city (pcity=0xce7280) at citytools.c:1285
pother_city = (struct city *) 0x0
powner = (struct player *) 0x93c018
pcenter = (struct tile *) 0xfae890
had_small_wonders = {vec = '\0' repeats 24 times}
old_vision = value optimized out
id = 378
had_great_wonders = false
__FUNCTION__ = remove_city
#5 0x00453780 in do_city_migration (pcity_from=0xce7280,
pcity_to=0xc69b90) at cityturn.c:2416
pplayer_from = (struct player *) 0x93c018
pplayer_to = (struct player *) 0x93c018
ptile_from = (struct tile *) 0xfae890
ptile_to = (struct tile *) 0xfb00f0
name_from = [l tgt=\tile\ x=11
y=17]Alexandroupolis[/l]\000/]\000\000(\\\217\002\000\000\000\000$\n\000\000\000\000\000\000,RZ\000\000\000\000\000�_U\000\000\000\000\000n��w|�\...@\000\000\000\000\000\000\000\000l\000\000\000\000\000\000\000\004\000\000\000\001\000\000\000\001\000\000\000\000\000\000\000�(\...@�(\237@
name_to = [l tgt=\city\ id=272 name=\Kavala\
/]\000\000\000\220\233�, '\0' repeats 13 times,
\220\233�\000\000\000\000\000\030�\223, '\0' repeats 13 times,
\220\233�\000\000\000\000\0004\000\000\000\000\000\000\000\220\233�\000\000\000\000\000Z,
'\0' repeats 15 times, \001\000\000\000\000\000\000
nation_from = 0xf70068 Hellenic
nation_to = 0xf70068 Hellenic
rcity = value optimized out
__FUNCTION__ = do_city_migration
Number 5 (do_city_migration()) can be different. I also got unit_enter_city()
and other functions calling remove_city() at this point.
I added a small condition after fc_assert_action() and it was executed every
time before the assertion error message was displayed
if (pcity-trade[o] == 0 || pother_city != NULL) {
log_error(should never happen);
}
Related question: I added an assert() call to fc_assert_fail() to get the
core dump. How can / should I debug this without adding assert() again?
___
Reply to this item at:
http://gna.org/bugs/?15498
___
Nachricht geschickt von/durch Gna!
http://gna.org/
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #15498] crash in remove_trade_route() - fc_assert_action()?
Follow-up Comment #1, bug #15498 (project freeciv): Related question: I added an assert() call to fc_assert_fail() to get the core dump. How can / should I debug this without adding assert() again? Start with _-F_ to raise the SIG_ABRT signal, or _-F sig_ to raise the signal you want. Notably, SIG_INT is fine when running with gdb, it allows you to continue after that. ___ Reply to this item at: http://gna.org/bugs/?15498 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #15499] Wrong handle of the illness_trade stuff
URL:
http://gna.org/bugs/?15499
Summary: Wrong handle of the illness_trade stuff
Project: Freeciv
Submitted by: pepeto
Submitted on: samedi 27.02.2010 à 13:30
Category: client
Severity: 3 - Normal
Priority: 5 - Normal
Status: None
Assigned to: None
Originator Email:
Open/Closed: Open
Release: S2_2
Discussion Lock: Any
Operating System: None
Planned Release: 2.2.1
___
Details:
if (has_capability(trade_illness, our_capability)) {
pcity-illness_trade = packet-illness_trade;
} else {
freelog(LOG_ERROR, Missing capability: 'trade_illness'.);
pcity-illness_trade = 0;
}
is the current part of the code. However, it is wrong on several points:
* The tests affect the client capability, which will always return TRUE
because we have this capability. The server would affect the server
capability (client.conn.capability).
* There shouldn't be an error message here, because connecting any 2.0 server
is legal. In that case (the server doesn't have the capability), the client
should read the illness field that has been sent.
___
Reply to this item at:
http://gna.org/bugs/?15499
___
Message posté via/par Gna!
http://gna.org/
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #15499] Wrong handle of the illness_trade stuff
Follow-up Comment #1, bug #15499 (project freeciv): This was cased by me applying the patch to fast. I will create a fix for it. The tests affect the client capability, which will always return TRUE because we have this capability. The server would affect the server capability (client.conn.capability). Is it enough to use 'client.conn.capability' in the test to do the right think? There shouldn't be an error message here, because connecting any 2.0 server is legal. In that case (the server doesn't have the capability), the client should read the illness field that has been sent. I will remove the error message. In the case that the server does not has the capability, illness due to trade is set to 0. The (old) illness field is not used at all by the client, because the client can calculate this value itself using the current data for the city (it should be up-to-date). As no data is known about the trade cities (turn_illness is not known), illness due to trade can not be calculated by the client. It has to be calculated within the server as done for trunk. The solution for S2_2 is to set illness due to trade to 0. The result matrix would be: old server - old client: * illness due to trade is used as part of the illness value within the server but not displayed in the client; the shown illness values do not add up to the overall illness old server - new client: * illness due to trade is used as part of the illness value within the server but not displayed in the client; the client sets the illness due to trade to 0 and the shown illness values do not add up to the overall illness new server - old client: * see 'old server - old client'; the new server sends the overall illness as expected by old clients new server - new client: * illness due to trade is calculated by the server and send to the client there it is displayed correctly; the optional network capability 'trade_illness' is needed I will now test all listed combinations ... ___ Reply to this item at: http://gna.org/bugs/?15499 ___ Nachricht geschickt von/durch Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #15499] Wrong handle of the illness_trade stuff
Follow-up Comment #2, bug #15499 (project freeciv): The tests affect the client capability, which will always return TRUE because we have this capability. The server would affect the server capability (client.conn.capability). Is it enough to use 'client.conn.capability' in the test to do the right think? our_capability refers to the capability of our side. So, if we are running the server, it is the server capability; if we are at client side, it refers to the client capability. The capability string of the connected entities is stored in the struct connection::capability. So, on server side, the capabilities of each client is stored in pconn-capability; on client side, the capability of the server is stored in client.conn.capability (client.conn is the connection to the server, formerly called aconn, such as in S2_1 branch). old server - old client This case doesn't make sense for us, because it occurs in old revisions. What we are doing is a new client connected to a new or old server; or new server connected by new or old clients. So the case old/old shouldn't be thought here. ___ Reply to this item at: http://gna.org/bugs/?15499 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #1222] function to access information for different radii
Update of patch #1222 (project freeciv): Status: In Progress = Ready For Test ___ Reply to this item at: http://gna.org/patch/?1222 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #15484] Improve translation sq. mi., M tons, M goods, bulbs
Follow-up Comment #4, bug #15484 (project freeciv): I think and I agree with you. If used abbreviation for M tons and M goods ( млн. тон and млн. товаров in Russia) the plural form isn't necessary. For sq. mi. needs plural form (1 кв. миля, 2 кв. мили or 5 кв. миль). There is no abbreviation for mile in Russian. ___ Reply to this item at: http://gna.org/bugs/?15484 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #15499] Wrong handle of the illness_trade stuff
Follow-up Comment #3, bug #15499 (project freeciv): still working on it. There are some strange things happening. I have compiled 2.2.0 and S2_2 with the patch and I'm trying to test the combination old server - new client. I added only an additional output line printing client.conn.capability to handle_chat_msg(). Somethere the server changed its capabilities ... ? Copied from the client message window: Client capability string: +2.2b trade_illness Server capability string: +2.2b Welcome to the Freeciv version 2.2.0+ Server running at mattsys port 9876. Server capability string [handle_chat_msg]: +2.2b Server capability string [handle_chat_msg]: +2.2b (server prompt): 'save testgame' Server capability string [handle_chat_msg]: +2.2b You are logged in as 'matthias' connected to no player. Server capability string [handle_chat_msg]: +2.2b trade_illness Established control over the server. You have command access level 'hack'. I'm a little bit lost at the moment about this combination * new server - new client OK; overall illness is the sum of the parts * new server - old client kind of OK; overall illness shown in the city dialog is OK (value from the server); illness shown in the popup is missing the illness due to trade as value and in the sum * old server - new client See the problem explained above; due to the change of the capability string the value within the packet is used as illness due to trade and the overall illness value is doubled. In this combination the value _should_ be ignored. Thus, within the city window and within the popup only the overall illness values minus trade illness would be displayed. old server - old client I know that this combination is not needed; but as reference it helps to show the changes done to the code. Attached my current patch for S2_2. (file #8309) ___ Additional Item Attachment: File name: health.diffSize:1 KB ___ Reply to this item at: http://gna.org/bugs/?15499 ___ Nachricht geschickt von/durch Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #15499] Wrong handle of the illness_trade stuff
Follow-up Comment #4, bug #15499 (project freeciv): Somethere the server changed its capabilities ... ? I don't think so. I cannot reproduce, but I think you connected an old server just before and you are using non-initialized datas. The capability string of the server is set in handle_server_join_reply(). ___ Reply to this item at: http://gna.org/bugs/?15499 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #15499] Wrong handle of the illness_trade stuff
Follow-up Comment #5, bug #15499 (project freeciv): Ouch, I found what is wrong... handle_conn_info() overwrite with the server capability with our capability... ___ Reply to this item at: http://gna.org/bugs/?15499 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #15500] do not change the status of the connection of the client to the server
URL: http://gna.org/bugs/?15500 Summary: do not change the status of the connection of the client to the server Project: Freeciv Submitted by: syntron Submitted on: Samstag 27.02.2010 um 20:04 Category: general Severity: 4 - Important Priority: 5 - Normal Status: None Assigned to: None Originator Email: Open/Closed: Open Release: 2.2.0 Discussion Lock: Any Operating System: None Planned Release: ___ Details: within handle_conn_info() the server capabilities are changed (see bug #15499); a hack to prevent updates of the server capability string is attached. If someone knows a better solution he is welcome This problem should (?) also be valid for trunk ___ File Attachments: --- Date: Samstag 27.02.2010 um 20:04 Name: 20100227-S2_2-do-not-change-the-status-of-the-connection-of-the-cl.patch Size: 920B By: syntron http://gna.org/bugs/download.php?file_id=8310 ___ Reply to this item at: http://gna.org/bugs/?15500 ___ Nachricht geschickt von/durch Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #15499] Wrong handle of the illness_trade stuff
Follow-up Comment #6, bug #15499 (project freeciv): the server capabilities are changed within handle_conn_info() (see bug #15500): If the patch for bug #15500 is applied, the combination old server - new client works as expected: the illness value minus trade is displayed within the city window as well as in the illness popup patch attached (file #8311) ___ Additional Item Attachment: File name: 20100227-S2_2-fix-health-capability-check.patch Size:0 KB ___ Reply to this item at: http://gna.org/bugs/?15499 ___ Nachricht geschickt von/durch Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #15499] Wrong handle of the illness_trade stuff
Update of bug #15499 (project freeciv): Depends on: = bugs #15500 ___ Reply to this item at: http://gna.org/bugs/?15499 ___ Nachricht geschickt von/durch Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #15499] Wrong handle of the illness_trade stuff
Follow-up Comment #7, bug #15499 (project freeciv): Ouch, I found what is wrong... handle_conn_info() overwrite with the server capability with our capability... I needed a little bit longer to validate this fact ;-) A hack to prevent this can be found in bug #15500 ___ Reply to this item at: http://gna.org/bugs/?15499 ___ Nachricht geschickt von/durch Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #15500] do not change the status of the connection of the client to the server
Follow-up Comment #1, bug #15500 (project freeciv): I would prefer the test if (pconn != client.conn). ___ Reply to this item at: http://gna.org/bugs/?15500 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #15500] do not change the status of the connection of the client to the server
Update of bug #15500 (project freeciv): Status:None = Ready For Test Planned Release: = 2.2.1, 2.3.0 ___ Follow-up Comment #2: I would prefer the test if (pconn != client.conn). Done. Furthermore all data of the connection is included within the condition. Patch for S2_2 and trunk (file #8312) ___ Additional Item Attachment: File name: version2-20100227-S2_2-do-not-change-the-status-of-the-connection-of-the-cl.patch Size:1 KB ___ Reply to this item at: http://gna.org/bugs/?15500 ___ Nachricht geschickt von/durch Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #15500] do not change the status of the connection of the client to the server
Follow-up Comment #3, bug #15500 (project freeciv): Ouch, this is very dangerous, the client won't know anymore what is its current state (e.g. player target, observing state, etc)... This item really need more work. ___ Reply to this item at: http://gna.org/bugs/?15500 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #15328] Asked for the name of a city but you can't fund it
Update of bug #15328 (project freeciv): Status: Ready For Test = Fixed Open/Closed:Open = Closed ___ Reply to this item at: http://gna.org/bugs/?15328 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #15468] map_is_known_and_seen segfault
Update of bug #15468 (project freeciv): Status: Ready For Test = Fixed Open/Closed:Open = Closed ___ Reply to this item at: http://gna.org/bugs/?15468 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #15469] Received packet PLAYER_READY from non-player connection
Update of bug #15469 (project freeciv): Status: Ready For Test = Fixed Open/Closed:Open = Closed ___ Follow-up Comment #6: Fix included by mistake on revision #16962 for the trunk branch. ___ Reply to this item at: http://gna.org/bugs/?15469 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #15480] Server start when connection leaves
Update of bug #15480 (project freeciv): Status: Ready For Test = Fixed Open/Closed:Open = Closed ___ Reply to this item at: http://gna.org/bugs/?15480 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #15456] api_actions_change_gold() crash
Update of bug #15456 (project freeciv): Status: Ready For Test = Fixed Open/Closed:Open = Closed ___ Reply to this item at: http://gna.org/bugs/?15456 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #1484] struct server_option
Update of patch #1484 (project freeciv): Privacy: Public = Private Open/Closed:Open = Closed ___ Reply to this item at: http://gna.org/patch/?1484 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [patch #1488] Remove g_return_if_fail
Update of patch #1488 (project freeciv): Status: Ready For Test = Done Open/Closed:Open = Closed ___ Reply to this item at: http://gna.org/patch/?1488 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #15347] Bad drawing and floating point exception with trade route line drawing in SDL client
Follow-up Comment #9, bug #15347 (project freeciv): I am sorry, I cannot reproduce this crash, even with 1680x1050 resolution. (I also noticed that long diagonal lines generated in stages during scrolling aren't pixel perfect; they look lumpy and occasionally don't join by a pixel or so. Maybe that's expected inaccuracy, I haven't tried to analyse the code at all.) This may the left problem. However, I don't have the required knowledge to investigate more in this direction. Already remembering me Thales theorem was already an exploit to me, as I stopped scientific studies very too early. ___ Reply to this item at: http://gna.org/bugs/?15347 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #15500] do not change the status of the connection of the client to the server
Follow-up Comment #4, bug #15500 (project freeciv): I don't know if I understand this code correct but here is my interpretation: the added condition prevents the client to change the data / information about the server. The server will never change, i.e. at least the address, capabilities and username of the connection to the server will be constant as long as this connection exists. So one patch in-between: only this values are not change for the client's connection to the server. I tested it doing different combinations of * (re)connect to the server as observer; leave * (re)connect; edit mode on / off; leave * (re)connect as player; leave * (re)connect using another user name; leave (file #8314) ___ Additional Item Attachment: File name: version3-20100227-S2_2-do-not-change-the-status-of-the-connection-of-the-cl.patch Size:1 KB ___ Reply to this item at: http://gna.org/bugs/?15500 ___ Nachricht geschickt von/durch Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #15477] Untranslated strings + wrong part of speech in scenario victory
Update of bug #15477 (project freeciv): Status: Ready For Test = In Progress ___ Reply to this item at: http://gna.org/bugs/?15477 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #15477] Untranslated strings + wrong part of speech in scenario victory
Update of bug #15477 (project freeciv): Status: In Progress = Fixed Open/Closed:Open = Closed ___ Reply to this item at: http://gna.org/bugs/?15477 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
