Bug #1676: hitpoints = 0 in units.ruleset causes divide by 0 crash - Freeciv - Freeciv Ticket System

Actions

Copy link

Bug #1676

open

hitpoints = 0 in units.ruleset causes divide by 0 crash

Added by Dean Brown about 2 months ago. Updated 8 days ago.

Status:

In Review

Priority:

Normal

Assignee:

Marko Lindqvist

Category:

Server

Target version:

3.2.2

Start date:

09/08/2025

Due date:

% Done:

Estimated time:

Description

Careless rule set hacking strikes again, but there's nothing warning you not to set hitpoints = 0 in units.ruleset, and the ruleset loads OK.

In tilespec.c routine fill_unit_sprite_array()

  ihp = ((NUM_TILES_HP_BAR-1)*punit->hp) / ptype->hp;

Files

Download all files

1676_S3_2.patch (4.36 KB) 1676_S3_2.patch		Dean Brown, 10/19/2025 02:31 AM
1676_S3_3.patch (4.41 KB) 1676_S3_3.patch		Dean Brown, 10/22/2025 04:48 AM
1676_Main.patch (4.41 KB) 1676_Main.patch		Dean Brown, 10/22/2025 04:51 AM

Actions

Copy link

Updated by Dean Brown about 2 months ago

File 1676.patch added

Patch should be good for all branches.

Actions

Copy link

Updated by Marko Lindqvist about 2 months ago

That's unlikely to be the only place where such a zombie (unit dies when it reach 0 HP) causes problems. The fix should be in ruleset sanity checking.

Actions

Copy link

Updated by Marko Lindqvist about 2 months ago

secfile_lookup_int_default_min_max() would be your friend.

Actions

Copy link

Updated by Dean Brown about 2 months ago

File deleted (~~1676.patch~~)

Actions

Copy link

Updated by Dean Brown about 2 months ago

File 1676_S3_2.patch added
File 1676_S3_3_Main.patch added

Agree, better to fix the cause instead of the symptom.

Actions

Copy link

Updated by Marko Lindqvist about 2 months ago

For rulesets that play excessively with better firepower and hitpoints as game progresses, maximum hp of 100 is rather low upper limit, especially in S3_2 where we should not break any existing rulesets (d3f). I'd say 10000 would be safe value even as such absolute maximum.

Actions

Copy link

Updated by Dean Brown about 2 months ago

File deleted (~~1676_S3_2.patch~~)

Actions

Copy link

Updated by Dean Brown about 2 months ago

File deleted (~~1676_S3_3_Main.patch~~)

Actions

Copy link

Updated by Dean Brown about 2 months ago

File 1676_S3_2.patch added
File 1676_S3_3_Main.patch added

I wasn't sure about what max value to set, thanks for the suggestion. Updated the patches.

Actions

Copy link

#10

Updated by Marko Lindqvist about 1 month ago

Category changed from client-common to Server
Target version set to 3.2.2

Actions

Copy link

#11

Updated by Marko Lindqvist 20 days ago

You should also make it impossible to set hitpoints to zero (or other illegal value) from the ruledit. Otherwise one may accidentally changes hitpoints to zero, saves the ruleset, and the ruleset load time check makes it impossible to load ruleset back in for fixing the situation.

Actions

Copy link

#12

Updated by Dean Brown 18 days ago

File 1676_ruledit.patch added

Good point. Turns out there's already code to set limits, just need to change the min value from 0 to 1.

1676_ruledit.patch good for all branches.

Actions

Copy link

#13

Updated by Marko Lindqvist 13 days ago

We don't want to knowingly introduce the bug in the git history, and only separately fix it. Please combine ruledit change with the rest of the patch (single patch for each branch)

Actions

Copy link

#14