[Freeciv-Dev] (PR#40020) Segfault in server aiunit.c ai_manage_units = plrhand.c maybe_make_contact()
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40020
I just ran it under S2_1 as:
$ ./ser -f ~/-0200x.sav -p 5557
...
set timeout 1
start
...
civserver: player.c:246: player_index: Assertion `pplayer' failed.
Aborted (core dumped)
Then the same under valgrind:
==5035== Invalid read of size 4
==5035==at 0x808A7B4: maybe_make_contact (plrhand.c:1206)
==5035==by 0x806D1E9: move_unit (unittools.c:2853)
==5035==by 0x80B11C4: handle_unit_move_request (unithand.c:1201)
==5035==by 0x813D2E8: ai_unit_move (aitools.c:1022)
==5035==by 0x813B69C: ai_unit_execute_path (aitools.c:194)
==5035==by 0x813BDFF: ai_follow_path (aitools.c:385)
==5035==by 0x813C10B: ai_unit_goto_constrained (aitools.c:459)
==5035==by 0x813C9B3: ai_unit_goto (aitools.c:790)
==5035==by 0x813BBA9: ai_gothere (aitools.c:317)
==5035==by 0x8141B85: ai_military_attack (aiunit.c:1755)
==5035==by 0x81423AF: ai_manage_military (aiunit.c:2078)
==5035==by 0x8142B09: ai_manage_unit (aiunit.c:2245)
==5035==by 0x8142F7F: ai_manage_units (aiunit.c:2351)
==5035==by 0x81343BE: ai_do_first_activities (aihand.c:426)
==5035==by 0x8054D25: ai_start_phase (srv_main.c:594)
==5035==by 0x8055124: begin_phase (srv_main.c:726)
==5035==by 0x80571D9: srv_running (srv_main.c:1816)
==5035==by 0x8057B87: srv_main (srv_main.c:2194)
==5035==by 0x804B195: main (civserver.c:258)
==5035== Address 0x4dd2e30 is 8 bytes inside a block of size 12 free'd
==5035==at 0x402365C: free (vg_replace_malloc.c:323)
==5035==by 0x8050488: genlist_unlink (genlist.c:141)
==5035==by 0x80C7B3F: unit_list_unlink (speclist.h:105)
==5035==by 0x80C7AD0: game_remove_unit (game.c:148)
==5035==by 0x8068E13: server_remove_unit (unittools.c:1374)
==5035==by 0x8069072: wipe_unit (unittools.c:1436)
==5035==by 0x8068014: bounce_unit (unittools.c:1044)
==5035==by 0x8068393: resolve_stack_conflicts (unittools.c:1086)
==5035==by 0x8068403: resolve_unit_stacks (unittools.c:1110)
==5035==by 0x8088937: update_players_after_alliance_breakup
(plrhand.c:457)
==5035==by 0x8088BB2: handle_diplomacy_cancel_pact (plrhand.c:546)
==5035==by 0x808A5C9: make_contact (plrhand.c:1180)
==5035==by 0x808A7F3: maybe_make_contact (plrhand.c:1207)
==5035==by 0x806D1E9: move_unit (unittools.c:2853)
==5035==by 0x80B11C4: handle_unit_move_request (unithand.c:1201)
==5035==by 0x813D2E8: ai_unit_move (aitools.c:1022)
==5035==by 0x813B69C: ai_unit_execute_path (aitools.c:194)
==5035==by 0x813BDFF: ai_follow_path (aitools.c:385)
==5035==by 0x813C10B: ai_unit_goto_constrained (aitools.c:459)
==5035==by 0x813C9B3: ai_unit_goto (aitools.c:790)
==5035==by 0x813BBA9: ai_gothere (aitools.c:317)
==5035==by 0x8141B85: ai_military_attack (aiunit.c:1755)
==5035==by 0x81423AF: ai_manage_military (aiunit.c:2078)
==5035==by 0x8142B09: ai_manage_unit (aiunit.c:2245)
==5035==by 0x8142F7F: ai_manage_units (aiunit.c:2351)
==5035==by 0x81343BE: ai_do_first_activities (aihand.c:426)
==5035==by 0x8054D25: ai_start_phase (srv_main.c:594)
==5035==by 0x8055124: begin_phase (srv_main.c:726)
==5035==by 0x80571D9: srv_running (srv_main.c:1816)
==5035==by 0x8057B87: srv_main (srv_main.c:2194)
==5035==by 0x804B195: main (civserver.c:258)
make_contact kills the unit because of the broken treaty and bouncing
(strange in itself but whatever). Then since maybe_make_contact doesn't
use a proper iterator it breaks the loop. Attached patch should fix it
for 2.1 and most likely 2.2/trunk.
-jason
Index: server/plrhand.c
===
--- server/plrhand.c (revision 14329)
+++ server/plrhand.c (working copy)
@@ -1203,9 +1203,9 @@
if (pcity) {
make_contact(pplayer, city_owner(pcity), ptile);
}
-unit_list_iterate(tile1-units, punit) {
+unit_list_iterate_safe(tile1-units, punit) {
make_contact(pplayer, unit_owner(punit), ptile);
-} unit_list_iterate_end;
+} unit_list_iterate_safe_end;
} square_iterate_end;
}
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40051) [Patch] LUA_CFLAGS
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40051 Instead of hardcoding lua tolua paths to Makefiles, they are defined by configure (hardcoded there as long as we are using our own lua source tree, trivial to replace with pkg-config check later) - ML diff -Nurd -X.diff_ignore freeciv/configure.ac freeciv/configure.ac --- freeciv/configure.ac 2008-01-25 22:45:32.0 +0200 +++ freeciv/configure.ac 2008-01-27 01:00:01.0 +0200 @@ -1,4 +1,4 @@ -dnl configure.ac for freeciv, for use with autoconf 2.50+ +dnl configure.ac for freeciv dnl Process this file with autoconf to produce a configure script. dnl Initialize with some random file to ensure the source is here. @@ -676,6 +676,28 @@ fi]) fi +dnl In the future we probably get rid of our own lua tree and use +dnl lua from system. +dnl LUA_AS_DEPENDENCY should be empty when using lua outside own our tree. +LUA_CFLAGS=-I\$(top_srcdir)/dependencies/lua/include +LUA_LIBS=\$(top_builddir)/dependencies/lua/src/liblua.a \ + \$(top_builddir)/dependencies/lua/src/lib/liblualib.a +LUA_AS_DEPENCY=$LUA_LIBS +AC_SUBST([LUA_CFLAGS]) +AC_SUBST([LUA_LIBS]) +AC_SUBST([LUA_AS_DEPENDENCY]) + + +TOLUA_CFLAGS=-I\$(top_srcdir)/dependencies/tolua +TOLUA_LIBS=\$(top_builddir)/dependencies/tolua/libtolua.a +TOLUA_AS_DEPENCY=$TOLUA_LIBS +AC_SUBST([TOLUA_CFLAGS]) +AC_SUBST([TOLUA_LIBS]) +AC_SUBST([TOLUA_AS_DEPENDENCY]) + +TOLUA=\$(top_builddir)/dependencies/tolua/tolua +AC_SUBST([TOLUA]) + dnl Freeciv uses a non-standard macro, Q_(), to handle cases of qualified dnl translatable strings and macro PL_() to handle plural forms. dnl Since the Gettext supplied Autoconf support diff -Nurd -X.diff_ignore freeciv/manual/Makefile.am freeciv/manual/Makefile.am --- freeciv/manual/Makefile.am 2007-09-19 12:30:22.0 +0300 +++ freeciv/manual/Makefile.am 2008-01-26 16:27:13.0 +0200 @@ -13,9 +13,7 @@ ../utility/libcivutility.a ../common/aicore/libaicore.a \ ../client/helpdata.o \ ../server/scripting/libscripting.a \ - ../dependencies/lua/src/liblua.a \ - ../dependencies/lua/src/lib/liblualib.a \ - ../dependencies/tolua/libtolua.a \ + $(LUA_AS_DEPENDENCY) $(TOLUA_AS_DEPENDENCY) \ ../server/generator/libgenerator.a civmanual_LDFLAGS = $(GGZDMOD_LDFLAGS) civmanual_LDADD= ../utility/libcivutility.a ../common/libcivcommon.a \ @@ -25,8 +23,6 @@ ../utility/libcivutility.a ../server/libcivserver.a \ ../utility/libcivutility.a ../common/aicore/libaicore.a \ ../server/scripting/libscripting.a \ - ../dependencies/lua/src/liblua.a \ - ../dependencies/lua/src/lib/liblualib.a \ - ../dependencies/tolua/libtolua.a \ + $(LUA_LIBS) $(TOLUA_LIBS) \ ../server/generator/libgenerator.a \ $(SERVER_LIBS) $(AUTH_LIBS) $(LIB_GGZDMOD) diff -Nurd -X.diff_ignore freeciv/server/Makefile.am freeciv/server/Makefile.am --- freeciv/server/Makefile.am 2007-10-29 23:26:28.0 +0200 +++ freeciv/server/Makefile.am 2008-01-26 16:23:44.0 +0200 @@ -91,9 +91,7 @@ ../ai/libcivai.a ../utility/libcivutility.a ./libcivserver.a \ ../utility/libcivutility.a ../common/aicore/libaicore.a \ ./scripting/libscripting.a \ - ../dependencies/lua/src/liblua.a \ - ../dependencies/lua/src/lib/liblualib.a \ - ../dependencies/tolua/libtolua.a \ + $(LUA_AS_DEPENDENCY) $(TOLUA_AS_DEPENDENCY) \ ./generator/libgenerator.a civserver_LDFLAGS = $(GGZDMOD_LDFLAGS) civserver_LDADD= ../utility/libcivutility.a ../common/libcivcommon.a \ @@ -102,9 +100,7 @@ ../utility/libcivutility.a ./libcivserver.a ../utility/libcivutility.a \ ../common/aicore/libaicore.a ./generator/libgenerator.a \ ./scripting/libscripting.a \ - ../dependencies/lua/src/liblua.a \ - ../dependencies/lua/src/lib/liblualib.a \ - ../dependencies/tolua/libtolua.a \ + $(LUA_LIBS) $(TOLUA_LIBS) \ $(AUTH_LIBS) $(SERVER_LIBS) $(LIB_GGZDMOD) $(SERVERICON) desktopfiledir = $(prefix)/share/applications diff -Nurd -X.diff_ignore freeciv/server/scripting/Makefile.am freeciv/server/scripting/Makefile.am --- freeciv/server/scripting/Makefile.am 2007-09-19 12:30:22.0 +0300 +++ freeciv/server/scripting/Makefile.am 2008-01-26 16:26:00.0 +0200 @@ -5,8 +5,7 @@ AM_CPPFLAGS = \ -I$(top_srcdir)/utility -I$(top_srcdir)/common \ -I$(top_srcdir)/server \ - -I$(top_srcdir)/dependencies/lua/include \ - -I$(top_srcdir)/dependencies/tolua + $(LUA_CFLAGS) $(TOLUA_CFLAGS) # api_gen.[ch] are now distributed to aid in cross-compiling. See PR#13571. dist_libscripting_a_SOURCES = \ @@ -34,5 +33,4 @@ api.pkg $(srcdir)/api_gen.c $(srcdir)/api_gen.h: $(srcdir)/api.pkg - $(top_builddir)/dependencies/tolua/tolua -n api -o $(srcdir)/api_gen.c -H $(srcdir)/api_gen.h $(srcdir)/api.pkg - + $(TOLUA) -n api -o $(srcdir)/api_gen.c -H $(srcdir)/api_gen.h $(srcdir)/api.pkg ___ Freeciv-dev
[Freeciv-Dev] (PR#40053) [patch] fix gui-win32 compilation
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40053 The attached patches fix gui-win32 compilation in S2_1 and S2_2. Will commit immediately. Index: client/gui-win32/happiness.c === --- client/gui-win32/happiness.c (revision 14320) +++ client/gui-win32/happiness.c (working copy) @@ -19,6 +19,7 @@ #include windowsx.h #include city.h +#include citydlg_common.h #include fcintl.h #include game.h #include government.h @@ -181,13 +182,13 @@ repaint_happiness_box(dlg,hdc); ReleaseDC(dlg-win,hdc); - SetWindowText(pdialog-mod_label[CITIES], + SetWindowText(dlg-mod_label[CITIES], text_happiness_cities(dlg-pcity)); SetWindowText(dlg-mod_label[LUXURIES], text_happiness_luxuries(dlg-pcity)); SetWindowText(dlg-mod_label[BUILDINGS], text_happiness_buildings(dlg-pcity)); - SetWindowText(pdialog-mod_label[UNITS], + SetWindowText(dlg-mod_label[UNITS], text_happiness_units(dlg-pcity)); SetWindowText(dlg-mod_label[WONDERS], text_happiness_wonders(dlg-pcity)); Index: client/gui-win32/menu.c === --- client/gui-win32/menu.c (revision 14320) +++ client/gui-win32/menu.c (working copy) @@ -24,6 +24,7 @@ #include astring.h #include capability.h #include fcintl.h +#include game.h #include log.h #include government.h #include map.h Index: client/gui-win32/happiness.c === --- client/gui-win32/happiness.c (revision 14320) +++ client/gui-win32/happiness.c (working copy) @@ -19,6 +19,7 @@ #include windowsx.h #include city.h +#include citydlg_common.h #include fcintl.h #include game.h #include government.h @@ -181,13 +182,13 @@ repaint_happiness_box(dlg,hdc); ReleaseDC(dlg-win,hdc); - SetWindowText(pdialog-mod_label[CITIES], + SetWindowText(dlg-mod_label[CITIES], text_happiness_cities(dlg-pcity)); SetWindowText(dlg-mod_label[LUXURIES], text_happiness_luxuries(dlg-pcity)); SetWindowText(dlg-mod_label[BUILDINGS], text_happiness_buildings(dlg-pcity)); - SetWindowText(pdialog-mod_label[UNITS], + SetWindowText(dlg-mod_label[UNITS], text_happiness_units(dlg-pcity)); SetWindowText(dlg-mod_label[WONDERS], text_happiness_wonders(dlg-pcity)); ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40028) gtk/pango invalid UTF-8 warning, fix documentation
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40028 I put the workaround into the doc/README -- and just (re-)discovered that's the *only* method described at: http://freeciv.wikia.com/wiki/Install-MacOSX#Localization_Support There's a different procedure at: http://freeciv.wikia.com/wiki/Install-Windows#Other_Languages There's nothing about running with LANG at: http://freeciv.wikia.com/wiki/Install Somebody needs to update these ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40052) [Patch] Simplify ser civ scripts
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40052 Minor cleanup for ser civ scripts. We already require new enough autoconf to depend on it to provide abs_top_srcdir and abs_top_builddir. autogen.sh set obsolete (otherwise unused) variable FC_USE_NEWAUTOCONF - ML diff -Nurd -X.diff_ignore freeciv/autogen.sh freeciv/autogen.sh --- freeciv/autogen.sh 2007-08-04 18:39:15.0 +0300 +++ freeciv/autogen.sh 2008-01-27 15:13:38.0 +0200 @@ -17,7 +17,6 @@ #DEBUG=defined FC_USE_NLS=yes -FC_USE_NEWAUTOCONF=yes FC_HELP=no # Leave out NLS checks diff -Nurd -X.diff_ignore freeciv/bootstrap/civ.in freeciv/bootstrap/civ.in --- freeciv/bootstrap/civ.in 2007-08-04 18:37:57.0 +0300 +++ freeciv/bootstrap/civ.in 2008-01-27 15:12:04.0 +0200 @@ -16,14 +16,10 @@ BUILDDIR=`dirname $0` -# Use cd + pwd instead of manual concatentation in case of absolute paths -BUILDDATA=$(cd $BUILDDIR/data ; pwd) -SRCDATA=$(cd $BUILDDIR ; cd @top_srcdir@/data ; pwd) - if [ x$FREECIV_PATH = x ] ; then FREECIV_PATH=.:data:~/.freeciv fi -export FREECIV_PATH=$FREECIV_PATH:$BUILDDATA:$SRCDATA +export FREECIV_PATH=$FREECIV_PATH:@abs_top_builddir@/data:@abs_top_srcdir@/data [ -x $BUILDDIR/client/civclient ] EXE=$BUILDDIR/client/civclient [ -x $BUILDDIR/civclient ] EXE=$BUILDDIR/civclient diff -Nurd -X.diff_ignore freeciv/bootstrap/ser.in freeciv/bootstrap/ser.in --- freeciv/bootstrap/ser.in 2007-08-04 18:37:57.0 +0300 +++ freeciv/bootstrap/ser.in 2008-01-27 15:12:39.0 +0200 @@ -16,14 +16,10 @@ BUILDDIR=`dirname $0` -# Use cd + pwd instead of manual concatentation in case of absolute paths -BUILDDATA=$(cd $BUILDDIR/data ; pwd) -SRCDATA=$(cd $BUILDDIR ; cd @top_srcdir@/data ; pwd) - if [ x$FREECIV_PATH = x ] ; then FREECIV_PATH=.:data:~/.freeciv fi -export FREECIV_PATH=$FREECIV_PATH:$BUILDDATA:$SRCDATA +export FREECIV_PATH=$FREECIV_PATH:@abs_top_builddir@/data:@abs_top_srcdir@/data [ -x $BUILDDIR/server/civserver ] EXE=$BUILDDIR/server/civserver [ -x $BUILDDIR/civserver ] EXE=$BUILDDIR/civserver ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#39857) attempt to bribe barbarian causes crash
URL: http://bugs.freeciv.org/Ticket/Display.html?id=39857
[EMAIL PROTECTED] - Fr 25. Jan 2008, 21:37:06]:
Version: Freeciv 2.1.2
Client crash bug 1: (SDL, Untested in GTK)
When trying to bribe an enemy unit, the client asks amount and
after
clicking yes, the client crashes. This has been earlier identified as
a bug
that happens when the window is closed and then information is
retreived from
the window without checking if it still exists. This was fixed for
GTK 2.1.0,
but appears not to have been fixed for SDL 2.1.2.
Actually it's still the same crash that you reported in PR#39716 a while
ago. The backport of the patch to S2_1 was unfortunately incomplete. Fix
attached.
Index: client/gui-sdl/diplomat_dialog.c
===
--- client/gui-sdl/diplomat_dialog.c (revision 14320)
+++ client/gui-sdl/diplomat_dialog.c (working copy)
@@ -1308,8 +1308,8 @@
static int diplomat_bribe_yes_callback(struct widget *pWidget)
{
if (Main.event.button.button == SDL_BUTTON_LEFT) {
-if (game_find_unit_by_number(pIncite_Dlg-diplomat_id)
-game_find_unit_by_number(pIncite_Dlg-diplomat_target_id)) {
+if (game_find_unit_by_number(pBribe_Dlg-diplomat_id)
+game_find_unit_by_number(pBribe_Dlg-diplomat_target_id)) {
request_diplomat_action(DIPLOMAT_BRIBE, pBribe_Dlg-diplomat_id,
pBribe_Dlg-diplomat_target_id, 0);
}
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] Windows GTK2 version in other languages
On Oct 31, 2007 9:01 PM, Christian Knoke [EMAIL PROTECTED] wrote: Stefan Hakspiel mailed me that, in order to use the GTK2 version 2.1 in german language, he had to set LANG=de *and* LANGUAGE=de in the environement, using a batch file. Otherwise, only the first start of Freeciv is in german. Christian Is this still the case? I can't test with german, but at least with LANG=fr everything seems to work fine here. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40054) Percentages in helpdata.c
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40054 I've noticed that, in the last remodelation of helpdata.c, for trunk and 2.2, three strings which contained 50%% have been changed to 50%. But two of them are still flagged as c-format, and msgfmt doesn't like them (The format specifications of argument 1 in msgid and msgstr are not the same, and msgstr is not a valid conversion specifier). The problematic strings are: * May be disbanded in a city to recover 50% of the production cost.\n * May fortify, granting a 50% defensive bonus.\n How do you remove the c-format? Joan Ive noticed that, in the last remodelation of helpdata.c, for trunk and 2.2, three strings which contained 50%% have been changed to 50%. But two of them are still flagged as c-format, and msgfmt doesnt like them (The format specifications of argument 1 in msgid and msgstr are not the same, and msgstr is not a valid conversion specifier). The problematic strings are:* May be disbanded in a city to recover 50% of the production cost.\n* May fortify, granting a 50% defensive bonus.\nHow do you remove the c-format? Joan ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40055) wrong chars in ruleset files
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40055 SVN 27 JAN 2008 S2_2 The use auf ' in terrain.ruleset causes some errors shown with 'make freeciv.pot'. The attached patch fixes these. Christian -- Christian Knoke* * *http://cknoke.de * * * * * * * * * Ceterum censeo Microsoft esse dividendum. Index: data/civ1/terrain.ruleset === --- data/civ1/terrain.ruleset (Revision 14332) +++ data/civ1/terrain.ruleset (Arbeitskopie) @@ -124,7 +124,7 @@ ;terrain section -- transformation changes to that terrain ; transform_time = time to transform; if 0, cannot transform ; warmer_wetter_result = result of global warming for wet terrains; one of: -;no -- no change; doesn't count for warming +;no -- no change; does not count for warming ;yes -- no change; counts for warming ;terrain section -- warming changes to that terrain ; warmer_drier_result = result of global warming for dry terrains; Index: data/civ2/terrain.ruleset === --- data/civ2/terrain.ruleset (Revision 14332) +++ data/civ2/terrain.ruleset (Arbeitskopie) @@ -132,7 +132,7 @@ ;terrain section -- transformation changes to that terrain ; transform_time = time to transform; if 0, cannot transform ; warmer_wetter_result = result of global warming for wet terrains; one of: -;no -- no change; doesn't count for warming +;no -- no change; does not count for warming ;yes -- no change; counts for warming ;terrain section -- warming changes to that terrain ; warmer_drier_result = result of global warming for dry terrains; Index: data/default/terrain.ruleset === --- data/default/terrain.ruleset (Revision 14332) +++ data/default/terrain.ruleset (Arbeitskopie) @@ -132,7 +132,7 @@ ;terrain section -- transformation changes to that terrain ; transform_time = time to transform; if 0, cannot transform ; warmer_wetter_result = result of global warming for wet terrains; one of: -;no -- no change; doesn't count for warming +;no -- no change; does not count for warming ;yes -- no change; counts for warming ;terrain section -- warming changes to that terrain ; warmer_drier_result = result of global warming for dry terrains; ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40054) Percentages in helpdata.c
Joan Creus wrote on Jan 27, 08:14 (-0800): URL: http://bugs.freeciv.org/Ticket/Display.html?id=40054 I've noticed that, in the last remodelation of helpdata.c, for trunk and 2.2, three strings which contained 50%% have been changed to 50%. But two of them are still flagged as c-format, and msgfmt doesn't like them (The format specifications of argument 1 in msgid and msgstr are not the same, and msgstr is not a valid conversion specifier). The problematic strings are: * May be disbanded in a city to recover 50% of the production cost.\n * May fortify, granting a 50% defensive bonus.\n How do you remove the c-format? More, lots of strings have 50 %. Maybe we decide between 50%% and 50 %? Christian -- Christian Knoke* * *http://cknoke.de * * * * * * * * * Ceterum censeo Microsoft esse dividendum. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40054) Percentages in helpdata.c
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40054 On 27/01/2008, Joan Creus wrote: How do you remove the c-format? Not really answering your question, but related news: I just today stumbled upon script fragment in the end of configure.ac that does some postprocessing for po/Makefile. One comment there says: Vast quantities of meaningless errors from xgettext is also annoying. We fix this by forcing xgettext to assume all files are C source files. I'll investigate this a bit more. - ML ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40056) [Patch] Configure checks for compiler commandline flags
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40056 - This adds new file compiler.m4 containing macros for testing if compiler supports some comamndline option or not. Main use for this will probably be setting tighter warning-options when compiler supports it instead of using only those options available in all gcc-versions we support. - Above macros are used for setting debug options (mainly Warning options) instead of just assuming that hardcoded set is supported in gcc, and no options set when using other compilers. - As side effect of the unification, C++ compilers are better supported. Originally this was written as part of #39661. The main principle remains, but otherwise couple of cleanup iterations has caused quite complete rewrite since then. - ML diff -Nurd -X.diff_ignore freeciv/configure.ac freeciv/configure.ac --- freeciv/configure.ac 2008-01-27 11:28:36.0 +0200 +++ freeciv/configure.ac 2008-01-27 19:23:30.0 +0200 @@ -327,15 +327,13 @@ AC_SUBST([POFILES]) fi -EXTRA_GCC_DEBUG_CFLAGS= -EXTRA_GCC_DEBUG_CXXFLAGS= - -if test -n $GCC; then - EXTRA_GCC_DEBUG_CFLAGS=$CFLAGS - EXTRA_GCC_DEBUG_CXXFLAGS=$CXXFLAGS - CFLAGS=-Wall -Wpointer-arith -Wcast-align -Wmissing-prototypes -Wmissing-declarations - CXXFLAGS=-Wall -Wpointer-arith -Wcast-align -fi +dnl Set debug flags supported by compiler +EXTRA_DEBUG_CFLAGS= +EXTRA_DEBUG_CXXFLAGS= +FC_C_FLAGS([-Wall -Wpointer-arith -Wcast-align -Wmissing-prototypes -Wmissing-declarations], + [], [EXTRA_DEBUG_CFLAGS]) +FC_CXX_FLAGS([-Wall -Wpointer-arith -Wcast-align], + [], [EXTRA_DEBUG_CXXFLAGS]) FC_DEBUG AC_C99_VARIADIC_MACROS @@ -742,10 +740,8 @@ fi fi]],[[]]) -if test -n $GCC; then - CFLAGS=$EXTRA_GCC_DEBUG_CFLAGS $CFLAGS - CXXFLAGS=$EXTRA_GCC_DEBUG_CXXFLAGS $CXXFLAGS -fi +CFLAGS=$EXTRA_DEBUG_CFLAGS $CFLAGS +CXXFLAGS=$EXTRA_DEBUG_CXXFLAGS $CXXFLAGS dnl Rebuild 'configure' whenever version.in changes, if maintainer mode enabled. AC_SUBST([CONFIGURE_DEPENDENCIES], [$CONFIGURE_DEPENDENCIES \$(top_srcdir)/version.in]) diff -Nurd -X.diff_ignore freeciv/m4/compiler.m4 freeciv/m4/compiler.m4 --- freeciv/m4/compiler.m4 1970-01-01 02:00:00.0 +0200 +++ freeciv/m4/compiler.m4 2008-01-27 19:11:41.0 +0200 @@ -0,0 +1,68 @@ +# Macros to check compiler options +# + +# Helper function that adds flags (words) to variable listing them. +# Makes sure there is no extra spaces even in any situation +# +# $1 - Name of the target variable +# $2 - Flags to add +# +AC_DEFUN([FC_ADD_WORDS_TO_VAR], +[ +old_value=`eval echo '$'$1` +if test x$old_value = x ; then + $1=$2 +elif test x$2 != x ; then + $1=$old_value $2 +fi +]) + +# Check if compiler supports given commandline parameter in language specific +# variable. If it does, it will be concatenated to variable. If several +# parameters are given, they are tested, and added to target variable, +# one at a time. +# +# $1 - Language +# $2 - Language specific variable +# $3 - Parameters to test +# $4 - Additional parameters +# $5 - Variable where to add +# + +AC_DEFUN([FC_COMPILER_FLAGS], +[ +AC_LANG_PUSH([$1]) + +flags_save=`eval echo '$'$2` +accepted_flags= + +for flag in $3 +do + $2=$flags_save $accepted_flags $flag $4 + AC_COMPILE_IFELSE([int a;], +[FC_ADD_WORDS_TO_VAR([accepted_flags], [$flag])]) +done +FC_ADD_WORDS_TO_VAR([$5], [$accepted_flags]) + +$2=$flags_save + +AC_LANG_POP([$1]) +]) + +# Commandling flag tests for C and C++ +# +# +# $1 - Parameters to test +# $2 - Additional parameters +# $3 - Variable where to add + +AC_DEFUN([FC_C_FLAGS], +[ +FC_COMPILER_FLAGS([C], [CFLAGS], [$1], [$2], [$3]) +]) + + +AC_DEFUN([FC_CXX_FLAGS], +[ +FC_COMPILER_FLAGS([C++], [CXXFLAGS], [$1], [$2], [$3]) +]) diff -Nurd -X.diff_ignore freeciv/m4/debug.m4 freeciv/m4/debug.m4 --- freeciv/m4/debug.m4 2007-08-04 18:36:07.0 +0300 +++ freeciv/m4/debug.m4 2008-01-27 19:16:02.0 +0200 @@ -10,12 +10,14 @@ dnl -g is added by AC_PROG_CC if the compiler understands it if test x$enable_debug = xyes; then - AC_DEFINE(DEBUG, 1, [Define if you want extra debugging.]) - EXTRA_GCC_DEBUG_CFLAGS=$EXTRA_GCC_DEBUG_CFLAGS -Werror + AC_DEFINE([DEBUG], [1], [Extra debugging support]) + FC_C_FLAGS([-Werror], [], [EXTRA_DEBUG_C_FLAGS]) + FC_CXX_FLAGS([-Werror], [], [EXTRA_DEBUG_CXX_FLAGS]) else if test x$enable_debug = xno; then -AC_DEFINE(NDEBUG, 1, [Define if you want no debug support.]) -EXTRA_GCC_DEBUG_CFLAGS=-O3 -fomit-frame-pointer +AC_DEFINE([NDEBUG], [1], [No debugging support at all]) +FC_C_FLAGS([-O3 -fomit-frame-pointer], [], [EXTRA_DEBUG_C_FLAGS]) +FC_CXX_FLAGS([-O3 -fomit-frame-pointer], [], [EXTRA_DEBUG_CXX_FLAGS]) fi fi ]) ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40020) Segfault in server aiunit.c ai_manage_units = plrhand.c maybe_make_contact()
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40020 Jason Short wrote: make_contact kills the unit because of the broken treaty and bouncing (strange in itself but whatever). Then since maybe_make_contact doesn't use a proper iterator it breaks the loop. Attached patch should fix it for 2.1 and most likely 2.2/trunk. That looks like it! Of course, it *was* a proper iterator right up until somebody added code to wipe an unrelated unit of another player that happens to be in a stack with a player that canceled a treaty In this case, the Indian player's last settler. Causing the Indian player to die! Good thing this is probably a rare event. So, let's consider this a temporary patch until bounce_unit() -- or its caller -- is fixed. What a terrible fragile logic mess! ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40054) Percentages in helpdata.c
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40054 Joan Creus wrote: I've noticed that, in the last remodelation of helpdata.c, for trunk and 2.2, three strings which contained 50%% have been changed to 50%. But two of them are still flagged as c-format, That's very odd. The other message doesn't have any problem: #: client/helpdata.c:1045 msgid * Must end turn in a city or next to land, or has a 50% risk of being lost at sea.\n The xgettext heuristic is guessing wrong for two cases. ... and msgfmt doesn't like them (The format specifications of argument 1 in msgid and msgstr are not the same, and msgstr is not a valid conversion specifier). I don't get this warning, but it sounds like a good idea! There are quite a few of these problems in *all* the po files. (PR#39970) The problematic strings are: * May be disbanded in a city to recover 50% of the production cost.\n * May fortify, granting a 50% defensive bonus.\n How do you remove the c-format? Some in data/helpdata.txt and various rulesets have: #, no-c-format They have: /* xgettext:no-c-format */ I'll add it immediately! Details at: http://www.gnu.org/software/gettext/manual/html_node/c_002dformat-Flag.html#c_002dformat-Flag ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40020) Segfault in server aiunit.c ai_manage_units = plrhand.c maybe_make_contact()
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40020 On 27/01/2008, William Allen Simpson wrote: So, let's consider this a temporary patch until bounce_unit() -- or its caller -- is fixed. OTOH. I have spent hundreds of hours during last three years hunting bugs that have turned out to be caused by unit dying inside unit_iterate(). Units can die quite unexpectedly, so I'd rather use _safe() in any high level iteration instead of wasting developer time in unnecessary bughunts. - ML ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40057) strerror-mystrerror
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40057
mystrerror is included in support.c. Two places don't use this but
access strerror directly. This patch fixes it (for 2.1).
-jason
Index: client/gui-sdl/gui_iconv.c
===
--- client/gui-sdl/gui_iconv.c (revision 14336)
+++ client/gui-sdl/gui_iconv.c (working copy)
@@ -208,7 +208,7 @@
size_t Res =
iconv(cd, (ICONV_CONST char **) pInptr, Insize, pOutptr, Outsize);
if (Res == (size_t) (-1)) {
-freelog(LOG_ERROR, iconv() error: %s, strerror(errno));
+freelog(LOG_ERROR, iconv() error: %s, mystrerror(errno));
if (errno == EINVAL) {
break;
} else {
Index: client/gui-mui/gui_main.c
===
--- client/gui-mui/gui_main.c (revision 14336)
+++ client/gui-mui/gui_main.c (working copy)
@@ -1085,7 +1085,7 @@
}
else if (sel 0)
{
- printf(%s\n, strerror(errno));
+ printf(%s\n, mystrerror(errno));
break;
}
}
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40028) gtk/pango invalid UTF-8 warning, fix documentation
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40028 William Allen Simpson wrote: URL: http://bugs.freeciv.org/Ticket/Display.html?id=40028 Jason Short wrote: # ... I thought I had written this up somewhere in the code, but I can find # nothing. Where should it be documented? Documenting the code and README.* files. Good, except, all rulesets must be (and to my knowledge are) in utf-8. Anything in latin1 will not be converted by the server and will be treated as utf-8. To my knowledge the only text files that can be something other than utf-8 are the PO files which give their own encoding. -jason ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40028) gtk/pango invalid UTF-8 warning, fix documentation
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40028
Here is a quick and partial fix. I assume that strerror() is one of the
most common offending functions, so I quickly went through and converted
all mystrerror users in client/ and server/ directories to use the
newly-written L_(). The only real issue with it is the use of
fixed-sized buffers which are wasteful if too large and broken if too
small, but as I said in my previous message I see no real alternative here.
Madeline, if you can test this and verify it fixes those strings then at
least we'll know we're getting somewhere. It would also be helpful if
you'd point out to us what other strings are being handled wrongly.
It is important to recognize which strings go in which encoding. If you
use printf() directly your strings must be in the local encoding. More
commonly freelog or fc_fprintf is used and these functions expect
strings in the internal encoding. Generally all freeciv functions
should expect the internal encoding unless specifically documented
otherwise.
Also as a side note there's another library we use, lua. Strings from
LUA are probably in the local encoding so should use L_ too.
-jason
Index: utility/fciconv.h
===
--- utility/fciconv.h (revision 14336)
+++ utility/fciconv.h (working copy)
@@ -19,6 +19,8 @@
#define FC_DEFAULT_DATA_ENCODING UTF-8
+#define L_(s, b) local_to_internal_string_buffer((s), (b), sizeof(b))
+
void init_character_encodings(const char *internal_encoding,
bool use_transliteration);
Index: server/meta.c
===
--- server/meta.c (revision 14336)
+++ server/meta.c (working copy)
@@ -44,6 +44,7 @@
#include connection.h
#include dataio.h
#include fcintl.h
+#include fciconv.h
#include log.h
#include mem.h
#include netintf.h
@@ -200,6 +201,7 @@
{
static char msg[8192];
static char str[8192];
+ char buf[512];
int rest = sizeof(str);
int n = 0;
char *s = str;
@@ -213,13 +215,14 @@
if ((sock = socket(AF_INET, SOCK_STREAM, 0)) == -1) {
freelog(LOG_ERROR, Metaserver: can't open stream socket: %s,
- mystrerror());
+ L_(mystrerror(), buf));
metaserver_failed();
return FALSE;
}
if (my_connect(sock, (struct sockaddr *) meta_addr, sizeof(meta_addr)) == -1) {
-freelog(LOG_ERROR, Metaserver: connect failed: %s, mystrerror());
+freelog(LOG_ERROR, Metaserver: connect failed: %s,
+ L_(mystrerror(), buf));
metaserver_failed();
my_closesocket(sock);
return FALSE;
Index: server/sernet.c
===
--- server/sernet.c (revision 14336)
+++ server/sernet.c (working copy)
@@ -806,7 +806,9 @@
fromlen = sizeof(fromend);
if ((new_sock = accept(sockfd, fromend.sockaddr, fromlen)) == -1) {
-freelog(LOG_ERROR, accept failed: %s, mystrerror());
+char buf[512];
+
+freelog(LOG_ERROR, accept failed: %s, L_(mystrerror(), buf));
return -1;
}
@@ -885,16 +887,17 @@
struct ip_mreq mreq;
const char *group;
int opt;
+ char buf[512];
/* Create socket for client connections. */
if((sock = socket(AF_INET, SOCK_STREAM, 0)) == -1) {
-die(socket failed: %s, mystrerror());
+die(socket failed: %s, L_(mystrerror(), buf));
}
opt=1;
if(setsockopt(sock, SOL_SOCKET, SO_REUSEADDR,
(char *)opt, sizeof(opt)) == -1) {
-freelog(LOG_ERROR, SO_REUSEADDR failed: %s, mystrerror());
+freelog(LOG_ERROR, SO_REUSEADDR failed: %s, L_(mystrerror(), buf));
}
if (!net_lookup_service(srvarg.bind_addr, srvarg.port, src)) {
@@ -904,23 +907,23 @@
}
if(bind(sock, src.sockaddr, sizeof (src)) == -1) {
-freelog(LOG_FATAL, bind failed: %s, mystrerror());
+freelog(LOG_FATAL, bind failed: %s, L_(mystrerror(), buf));
exit(EXIT_FAILURE);
}
if(listen(sock, MAX_NUM_CONNECTIONS) == -1) {
-freelog(LOG_FATAL, listen failed: %s, mystrerror());
+freelog(LOG_FATAL, listen failed: %s, L_(mystrerror(), buf));
exit(EXIT_FAILURE);
}
/* Create socket for server LAN announcements */
if ((socklan = socket(AF_INET, SOCK_DGRAM, 0)) 0) {
- freelog(LOG_ERROR, socket failed: %s, mystrerror());
+freelog(LOG_ERROR, socket failed: %s, L_(mystrerror(), buf));
}
if (setsockopt(socklan, SOL_SOCKET, SO_REUSEADDR,
(char *)opt, sizeof(opt)) == -1) {
-freelog(LOG_ERROR, SO_REUSEADDR failed: %s, mystrerror());
+freelog(LOG_ERROR, SO_REUSEADDR failed: %s, L_(mystrerror(), buf));
}
my_nonblock(socklan);
@@ -933,7 +936,7 @@
addr.sockaddr_in.sin_port = htons(SERVER_LAN_PORT);
if (bind(socklan, addr.sockaddr, sizeof(addr)) 0) {
-freelog(LOG_ERROR, bind failed: %s, mystrerror());
+freelog(LOG_ERROR, bind failed: %s, L_(mystrerror(), buf));
}
mreq.imr_multiaddr.s_addr = inet_addr(group);
@@ -941,7 +944,7 @@
if
[Freeciv-Dev] (PR#40032) server/plrhand.c civil war message plural
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40032 Committed as part of trunk revision 14337. Committed as part of S2_2 revision 14338. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40054) Percentages in client/helpdata.c
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40054 Committed as part of trunk revision 14337. Committed as part of S2_2 revision 14338. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40032) server/plrhand.c civil war message plural
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40032
Probably should attach the patch as applied to trunk for posterity!
(Also in 2.2, an update-po and vigorous line-by-line po massaging.)
Index: server/citytools.c
===
--- server/citytools.c (revision 14336)
+++ server/citytools.c (working copy)
@@ -730,7 +730,6 @@
Create a palace in a random city. Used when the capital was conquered.
**/
static void build_free_small_wonders(struct player *pplayer,
-const char *const old_capital_name,
bv_imprs *had_small_wonders)
{
int size = city_list_size(pplayer-cities);
@@ -742,12 +741,11 @@
improvement_iterate(pimprove) {
if (BV_ISSET(*had_small_wonders, improvement_index(pimprove))) {
- struct city *pnew_city;
+ /* FIXME: instead, find central city */
+ struct city *pnew_city = city_list_get(pplayer-cities, myrand(size));
assert(find_city_from_small_wonder(pplayer, pimprove) == NULL);
- pnew_city = city_list_get(pplayer-cities, myrand(size));
-
city_add_improvement(pnew_city, pimprove);
pplayer-small_wonders[improvement_index(pimprove)] = pnew_city-id;
@@ -757,9 +755,10 @@
*/
send_player_cities(pplayer);
- notify_player(pplayer, pnew_city-tile, E_CITY_LOST,
- _(You lost %s. A new %s was built in %s.),
- old_capital_name,
+ notify_player(pplayer, pnew_city-tile, E_IMP_BUILD,
+ /* FIXME: should already be notified about city loss? */
+ /* TRANS: building ... city */
+ _(A replacement %s was built in %s.),
improvement_name_translation(pimprove),
city_name(pnew_city));
/*
@@ -938,7 +937,7 @@
/* Build a new palace for free if the player lost her capital and
savepalace is on. */
if (game.info.savepalace) {
-build_free_small_wonders(pgiver, city_name(pcity), had_small_wonders);
+build_free_small_wonders(pgiver, had_small_wonders);
}
/* Remove the sight points from the giver...and refresh the city's
@@ -1096,7 +1095,6 @@
struct player *pplayer = city_owner(pcity);
struct tile *ptile = pcity-tile;
bv_imprs had_small_wonders;
- char *cityname = mystrdup(city_name(pcity));
struct vision *old_vision;
int id = pcity-id; /* We need this even after memory has been freed */
@@ -1214,11 +1212,9 @@
/* Build a new palace for free if the player lost her capital and
savepalace is on. */
if (game.info.savepalace) {
-build_free_small_wonders(pplayer, cityname, had_small_wonders);
+build_free_small_wonders(pplayer, had_small_wonders);
}
- free(cityname);
-
sync_cities();
}
Index: server/plrhand.c
===
--- server/plrhand.c(revision 14336)
+++ server/plrhand.c(working copy)
@@ -1666,8 +1666,9 @@
if (player_count() = MAX_NUM_PLAYERS) {
/* No space to make additional player */
-freelog(LOG_NORMAL, _(Could not throw %s into civil war - too many
-players), player_name(pplayer));
+freelog(LOG_NORMAL,
+_(Could not throw %s into civil war - too many players),
+nation_plural_for_player(pplayer));
return;
}
@@ -1682,14 +1683,18 @@
/* Now split the empire */
freelog(LOG_VERBOSE,
- %s nation is thrust into civil war, created AI player %s,
+ %s civil war; created AI %s,
nation_rule_name(nation_of_player(pplayer)),
- player_name(cplayer));
+ nation_rule_name(nation_of_player(cplayer)));
notify_player(pplayer, NULL, E_CIVIL_WAR,
- _(Your nation is thrust into civil war,
- %s is declared the leader of the rebel states.),
- player_name(cplayer));
+_(Your nation is thrust into civil war.));
+ notify_player(pplayer, NULL, E_FIRST_CONTACT,
+/* TRANS: leader ... the Poles. */
+_(%s is the rebellious leader of the %s.),
+player_name(cplayer),
+nation_plural_for_player(cplayer));
+
i = city_list_size(pplayer-cities)/2; /* number to flip */
j = city_list_size(pplayer-cities); /* number left to process */
city_list_iterate(pplayer-cities, pcity) {
@@ -1703,35 +1708,35 @@
resolved stack conflicts for each city we would teleport the first
of the units we met since the other would have another owner */
transfer_city(cplayer, pcity, -1, FALSE, FALSE, FALSE);
- freelog(LOG_VERBOSE, %s declares allegiance to %s,
+ freelog(LOG_VERBOSE, %s declares allegiance to the %s.,
city_name(pcity),
- player_name(cplayer));
+
[Freeciv-Dev] (PR#40028) gtk/pango invalid UTF-8 warning, fix documentation
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40028 [jdorje - Sun Jan 27 22:16:24 2008]: Here is a quick and partial fix. I assume that strerror() is one of the most common offending functions, so I quickly went through and converted all mystrerror users in client/ and server/ directories to use the newly-written L_(). The only real issue with it is the use of fixed-sized buffers which are wasteful if too large and broken if too small, but as I said in my previous message I see no real alternative here. Might I suggest that instead of wrapping every call to mystrerror with L, you instead call L inside mysterror (and of course document that mystrerror does the conversion)? Or make a mystrerror_l that does this (to keep mysterror for the cases that shouldn't be converted). It would make your patch much simpler. I agree in general with your reservations about fixed size buffers, but I think that in this case it is acceptable to used them because: 1. It is very unlikey that any strerror message is longer than 256 characters (I have looked at _sys_errlist_internal in libc and that is the case, I assume it is very probably so on other platforms). 2. Strerror is not re-entrant (e.g. thread-safe) anyway (we would need to use strerror_r for that). 3. It is very unlikely that someone will need to call mysterror twice in the same expression (e.g. more than once in a printf-like call). Hence a fixed static buffer inside mystrerror (or mysterror_l) to hold the converted text would be an acceptable solution in this case, in my opinion. Madeline, if you can test this and verify it fixes those strings then at least we'll know we're getting somewhere. It would also be helpful if you'd point out to us what other strings are being handled wrongly. All the corrupted strings that I had found previously now display correctly (and no gtk warnings appear) with your patch applied. I cannot of course guarantee that that is all of them. ;) It is important to recognize which strings go in which encoding. If you use printf() directly your strings must be in the local encoding. More commonly freelog or fc_fprintf is used and these functions expect strings in the internal encoding. Generally all freeciv functions should expect the internal encoding unless specifically documented otherwise. Also as a side note there's another library we use, lua. Strings from LUA are probably in the local encoding so should use L_ too. I agree with you on those points. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40028) gtk/pango invalid UTF-8 warning, fix documentation
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40028 Jason Short wrote: Here is a quick and partial fix. I assume that strerror() is one of the most common offending functions, so I quickly went through and converted all mystrerror users in client/ and server/ directories to use the newly-written L_(). Now, I find this irritating on a number of levels. 1) This ticket was already patched and resolved. Another new ticket would be better. I'll open one. 2) Madeline reported the problem was in mystrerror(), so that's a good target, but the solution would be to patch mystrerror(), not repeatedly patch every invocation This is *all* users! 3) By definition, mystrerror() can only be called once, so there's no technical reason to use a dynamic buffer. 4) I really hate L_() hiding a simple single function. There's no excuse. This makes the code harder to read and harder to grep. 5) The parameter order doesn't match either cat_snprintf() or strlcat(), or any standard C invocation. By convention, the destination buffer is always the first parameter. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40058) mystrerror() and local encoding
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40058 Followup of PR#40028. See earlier patch and objections there. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40028) gtk/pango invalid UTF-8 warning, fix documentation
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40028 Jason Dorje Short wrote: Good, except, all rulesets must be (and to my knowledge are) in utf-8. Perhaps, but that's not what the data currently said, and I'd prefer to document reality rather than hope. Maybe there should be a concerted effort to scan every data file and ensure they are all UTF-8? ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40028) gtk/pango invalid UTF-8 warning, fix documentation
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40028 [jdorje - Sun Jan 27 22:16:24 2008]: Here is a quick and partial fix. I assume that strerror() is one of the most common offending functions, so I quickly went through and converted all mystrerror users in client/ and server/ directories to use the newly-written L_(). The only real issue with it is the use of fixed-sized buffers which are wasteful if too large and broken if too small, but as I said in my previous message I see no real alternative here. Might I suggest that instead of wrapping every call to mystrerror with L, you instead call L inside mysterror (and of course document that mystrerror does the conversion)? Or make a mystrerror_l that does this (to keep mysterror for the cases that shouldn't be converted). It would make your patch much simpler. I agree in general with your reservations about fixed size buffers, but I think that in this case it is acceptable to used them because: 1. It is very unlikey that any strerror message is longer than 256 characters (I have looked at _sys_errlist_internal in libc and that is the case, I assume it is very probably so on other platforms). 2. Strerror is not re-entrant (e.g. thread-safe) anyway (we would need to use strerror_r for that). 3. It is very unlikely that someone will need to call mysterror twice in the same expression (e.g. more than once in a printf-like call). Hence a fixed static buffer inside mystrerror (or mysterror_l) to hold the converted text would be an acceptable solution in this case, in my opinion. Madeline, if you can test this and verify it fixes those strings then at least we'll know we're getting somewhere. It would also be helpful if you'd point out to us what other strings are being handled wrongly. All the corrupted strings that I had found previously now display correctly (and no gtk warnings appear) with your patch applied. I cannot of course guarantee that that is all of them. ;) It is important to recognize which strings go in which encoding. If you use printf() directly your strings must be in the local encoding. More commonly freelog or fc_fprintf is used and these functions expect strings in the internal encoding. Generally all freeciv functions should expect the internal encoding unless specifically documented otherwise. Also as a side note there's another library we use, lua. Strings from LUA are probably in the local encoding so should use L_ too. I agree with you on those points. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40028) gtk/pango invalid UTF-8 warning, fix documentation
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40028 William Allen Simpson wrote: URL: http://bugs.freeciv.org/Ticket/Display.html?id=40028 Jason Short wrote: Here is a quick and partial fix. I assume that strerror() is one of the most common offending functions, so I quickly went through and converted all mystrerror users in client/ and server/ directories to use the newly-written L_(). Now, I find this irritating on a number of levels. 1) This ticket was already patched and resolved. Another new ticket would be better. I'll open one. What would possibly motivate you to resolve a bug that's still unfixed, mere hours after making a rant about how writing off bugs as acceptable is sloppy bugfixing? 2) Madeline reported the problem was in mystrerror(), so that's a good target, Ahh yes, I see that now. Obviously I had only remembered it subconsciously before :). Speaking of which, Madeline, what is the mystrsocketerror function you mention? but the solution would be to patch mystrerror(), not repeatedly patch every invocation This is *all* users! 3) By definition, mystrerror() can only be called once, so there's no technical reason to use a dynamic buffer. Yes. Naturally the first thing I considered was making the change directly within mystrerror, as that is far far easier. But, if one mystrerror call is done before the previous one's return value has been discarded the result will be erroneous. Given that some of the returned strings are passed off into arbitrary callbacks this is a rather risky thing to assume IMO. However the error if this happened would be non-fatal so I suppose if we want to accept that bug it's okay. 4) I really hate L_() hiding a simple single function. There's no excuse. This makes the code harder to read and harder to grep. 5) The parameter order doesn't match either cat_snprintf() or strlcat(), or any standard C invocation. By convention, the destination buffer is always the first parameter. It aims at consistency with _, N_, PL_, and Q_, with which it shares similar naming. All of these are just shortened wrappers for single functions that allow easy and common use, and all put the text parameter first. Of course L_ does no translation, it just changes encoding (as do the other _ functions). -jason ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40028) gtk/pango invalid UTF-8 warning, fix documentation
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40028 [EMAIL PROTECTED] - Mon Jan 28 05:22:53 2008]: William Allen Simpson wrote: ... 2) Madeline reported the problem was in mystrerror(), so that's a good target, Ahh yes, I see that now. Obviously I had only remembered it subconsciously before :). Speaking of which, Madeline, what is the mystrsocketerror function you mention? It is something added in the warclient codebase to deal with the fact that on win32 strerror/GetLastError does not return error codes for socket operation errors. It is identical to mystrerror except that the call to GetLastError is replaced by a call to WSAGetLastError. It's not really a pressing issue for the network code in the freeciv codebase; I had concocted a characteristically over-done asynchronous networking subsystem for metaserver communications for the client for which to debug (plagued by all manner of difficult to track down heisenbugs) on win32 it was necessary to have precise error reporting. In the end it was a nice learning experience, but not terribly worthwhile for the project as a whole. ;) 3) By definition, mystrerror() can only be called once, so there's no technical reason to use a dynamic buffer. Yes. Naturally the first thing I considered was making the change directly within mystrerror, as that is far far easier. But, if one mystrerror call is done before the previous one's return value has been discarded the result will be erroneous. Given that some of the returned strings are passed off into arbitrary callbacks this is a rather risky thing to assume IMO. However the error if this happened would be non-fatal so I suppose if we want to accept that bug it's okay. I don't see how an invocation of mystrerror could trigger another invocation of itself (I certainly wouldn't expect that to happen). As far as I can tell mystrerror is only used in calls to printf like functions. So the return value of mystrerror is immediately copied to the buffers in those functions and once there, it is safe from being corrupted by further calls to mystrerror. But if there are indeed complicated callbacks that just store the pointer returned by strerror and use it later on after other operations have been performed, then indeed, that is a dangerous situation and should be avoided. I would always put a big warning in the comment header for functions returning pointers to static data that the programmer should take care not to abuse the return value in this way. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
