From 20d1945d5d32d2daa90b77fddff25cc8f8d957d1 Mon Sep 17 00:00:00 2001 From: Alina Lenk Date: Thu, 18 Apr 2024 14:30:16 +0200 Subject: [PATCH 3/3] Network protocol: strvec field type Requested by Sveinung Kvilhaugsvik See HRM #657288 Signed-off-by: Alina Lenk --- common/generate_packets.py | 201 +++++++++++++++++++++++++++++-- common/networking/packets.def | 27 ++--- common/networking/packets.h | 23 ++-- common/networking/packets_json.h | 23 ++-- fc_version | 2 +- server/ruleset/ruleload.c | 22 ++-- 6 files changed, 234 insertions(+), 64 deletions(-) diff --git a/common/generate_packets.py b/common/generate_packets.py index 0d82858106..6c71073137 100755 --- a/common/generate_packets.py +++ b/common/generate_packets.py @@ -1154,7 +1154,7 @@ class StringType(SizedType): raise ValueError("not a valid string type") if public_type != "char": - raise ValueError(f"string dataio type with non-char public type: {public_type!r}") + raise ValueError(f"string type with illegal public type: {public_type!r}") super().__init__(dataio_type, public_type, size) @@ -1180,8 +1180,6 @@ if (!DIO_GET({self.dataio_type}, &din, &field_addr, real_packet->{location}, siz }} """ -DEFAULT_REGISTRY.dataio_types["string"] = DEFAULT_REGISTRY.dataio_types["estring"] = partial(NeedSizeType, cls = StringType) - class MemoryType(SizedType): """Type information for a memory field""" @@ -1253,9 +1251,15 @@ class ArrayType(FieldType): ) def get_code_handle_param(self, location: Location) -> str: - # add "const" if top level - pre = "" if location.depth else "const " - return pre + self.elem.get_code_handle_param(location.deeper(f"*{location}")) + # Note: If we're fine with writing `foo_t const *fieldname`, + # we'd only need one case, .deeper(f"const *{location}") + if not location.depth: + # foo_t fieldname ~> const foo_t *fieldname + return "const " + self.elem.get_code_handle_param(location.deeper(f"*{location}")) + else: + # const foo_t *fieldname ~> const foo_t *const *fieldname + # the final * is already part of {location} + return self.elem.get_code_handle_param(location.deeper(f"*const {location}")) def get_code_init(self, location: Location) -> str: if not self.complex: @@ -1278,7 +1282,6 @@ class ArrayType(FieldType): # can't use direct assignment to bit-copy a raw array, # even if our type is not complex inner_copy = prefix(" ", self.elem.get_code_copy(location.sub, dest, src)) - # FIXME: can't use self.size.real; have to use actual_for(src context) return f"""\ {{ int {location.index}; @@ -1557,6 +1560,186 @@ differ = FALSE; return f"{self.elem}[{self.size}]" +class StrvecType(FieldType): + """Type information for a string vector field""" + + dataio_type: str + """How fields of this type are transmitted over network""" + + public_type: str + """How fields of this type are represented in C code""" + + complex: bool = True + + def __init__(self, dataio_type: str, public_type: str): + if dataio_type not in ("string", "estring"): + raise ValueError("not a valid strvec type") + + if public_type != "struct strvec": + raise ValueError(f"strvec type with illegal public type: {public_type!r}") + + self.dataio_type = dataio_type + self.public_type = public_type + + def get_code_declaration(self, location: Location) -> str: + return f"""\ +{self.public_type} *{location}; +""" + + def get_code_handle_param(self, location: Location) -> str: + if not location.depth: + return f"const {self.public_type} *{location}" + else: + # const struct strvec *const *fieldname + # the final * is already part of {location} + # initial const gets added from outside + return f"{self.public_type} *const {location}" + + def get_code_init(self, location: Location) -> str: + # we're always allocating our vectors, even if they're empty + return f"""\ +{location} = strvec_new(); +""" + + def get_code_fill(self, location: Location) -> str: + """Generate a code snippet shallow-copying a value of this type from + dsend arguments into a packet struct.""" + # safety: the packet's contents will not be modified without cloning + # it first, so discarding 'const' qualifier here is safe + return f"""\ +real_packet->{location} = (struct strvec *) {location}; +""" + + def get_code_copy(self, location: Location, dest: str, src: str) -> str: + # dest is initialized by us ~> not null + # src might be a packet passed in from outside ~> could be null + return f"""\ +if ({src}->{location}) {{ + strvec_copy({dest}->{location}, {src}->{location}); +}} else {{ + strvec_clear({dest}->{location}); +}} +""" + + def get_code_free(self, location: Location) -> str: + return f"""\ +if ({location}) {{ + strvec_destroy({location}); + {location} = nullptr; +}} +""" + + def get_code_hash(self, location: Location) -> str: + raise ValueError(f"hash not supported for strvec type {self} in field {location.name}") + + def get_code_cmp(self, location: Location) -> str: + return f"""\ +differ = !are_strvecs_equal(old->{location}, real_packet->{location}); +""" + + def _get_code_put_full(self, location: Location) -> str: + # Note: strictly speaking, we could allow size == MAX_UINT16, + # but we might want to use that in the future to signal overlong + # vectors (like with jumbo packets) + # Though that would also mean packets larger than 64 KiB, + # which we're a long way from + return f"""\ +if (!real_packet->{location}) {{ + /* Transmit null vector as empty vector */ + e |= DIO_PUT(arraylen, &dout, &field_addr, 0); +}} else {{ + int {location.index}; + + fc_assert(strvec_size(real_packet->{location}) < MAX_UINT16); + e |= DIO_PUT(arraylen, &dout, &field_addr, strvec_size(real_packet->{location})); + +#ifdef FREECIV_JSON_CONNECTION + {location.json_subloc} = plocation_elem_new(0); +#endif /* FREECIV_JSON_CONNECTION */ + + for ({location.index} = 0; {location.index} < strvec_size(real_packet->{location}); {location.index}++) {{ + const char *pstr = strvec_get(real_packet->{location}, {location.index}); + + if (!pstr) {{ + /* Transmit null strings as empty strings */ + pstr = ""; + }} + +#ifdef FREECIV_JSON_CONNECTION + /* Next array element */ + {location.json_subloc}->number = {location.index}; +#endif /* FREECIV_JSON_CONNECTION */ + + e |= DIO_PUT({self.dataio_type}, &dout, &field_addr, pstr); + }} + +#ifdef FREECIV_JSON_CONNECTION + FC_FREE({location.json_subloc}); +#endif /* FREECIV_JSON_CONNECTION */ +}} +""" + + def get_code_put(self, location: Location, deep_diff: bool = False) -> str: + assert not deep_diff, "deep-diff for strvec not supported yet" + return self._get_code_put_full(location) + + def _get_code_get_full(self, location: Location) -> str: + return f"""\ +{{ + int {location.index}; + + if (!DIO_GET(arraylen, &din, &field_addr, &{location.index})) {{ + RECEIVE_PACKET_FIELD_ERROR({location.name}); + }} + strvec_reserve(real_packet->{location}, {location.index}); + +#ifdef FREECIV_JSON_CONNECTION + {location.json_subloc} = plocation_elem_new(0); +#endif /* FREECIV_JSON_CONNECTION */ + + for ({location.index} = 0; {location.index} < strvec_size(real_packet->{location}); {location.index}++) {{ + char readin[MAX_LEN_PACKET]; + +#ifdef FREECIV_JSON_CONNECTION + /* Next array element */ + {location.json_subloc}->number = {location.index}; +#endif /* FREECIV_JSON_CONNECTION */ + + if (!DIO_GET({self.dataio_type}, &din, &field_addr, readin, sizeof(readin)) + || !strvec_set(real_packet->{location}, {location.index}, readin)) {{ + RECEIVE_PACKET_FIELD_ERROR({location.name}); + }} + }} + +#ifdef FREECIV_JSON_CONNECTION + FC_FREE({location.json_subloc}); +#endif /* FREECIV_JSON_CONNECTION */ +}} +""" + + def get_code_get(self, location: Location, deep_diff: bool = False) -> str: + assert not deep_diff, "deep-diff for strvec not supported yet" + return self._get_code_get_full(location) + + def __str__(self) -> str: + return f"{self.dataio_type}({self.public_type})" + + +def string_type_ctor(dataio_type: str, public_type: str) -> RawFieldType: + """Field type constructor for both strings and string vectors""" + if dataio_type not in ("string", "estring"): + raise ValueError(f"not a valid string type: {dataio_type}") + + if public_type == "char": + return NeedSizeType(dataio_type, public_type, cls = StringType) + elif public_type == "struct strvec": + return StrvecType(dataio_type, public_type) + else: + raise ValueError(f"public type {public_type} not legal for dataio type {dataio_type}") + +DEFAULT_REGISTRY.dataio_types["string"] = DEFAULT_REGISTRY.dataio_types["estring"] = string_type_ctor + + class Field: """A single field of a packet. Consists of a name, type information (including array sizes) and flags.""" @@ -1699,7 +1882,7 @@ class Field: def get_init(self) -> str: """Generate code initializing this field in the packet struct, after the struct has already been zeroed.""" - return self.type_info.get_code_init(Location(self.name)) + return self.type_info.get_code_init(Location(self.name, f"packet->{self.name}")) def get_copy(self, dest: str, src: str) -> str: """Generate code deep-copying this field from *src to *dest.""" @@ -1713,7 +1896,7 @@ class Field: def get_free(self) -> str: """Generate code deinitializing this field in the packet struct before destroying the packet.""" - return self.type_info.get_code_free(Location(self.name)) + return self.type_info.get_code_free(Location(self.name, f"packet->{self.name}")) def get_hash(self) -> str: """Generate code factoring this field into a hash computation.""" diff --git a/common/networking/packets.def b/common/networking/packets.def index db1fd147b3..dbb8be3435 100644 --- a/common/networking/packets.def +++ b/common/networking/packets.def @@ -197,10 +197,7 @@ type MEMORY = memory(unsigned char) type REQUIREMENT = requirement(struct requirement) type ACT_PROB = action_probability(struct act_prob) type STRING = string(char) -# A string vector encoded to a string outside the packet and field system. -# Marking it this way is useful as documentation. The marking can also be -# used in non vanilla generate_packets.py packet generators. -type STRVEC = STRING +type STRVEC = string(struct strvec) type WORKLIST = worklist(struct worklist) # string that is URI encoded in the JSON protocol type ESTRING = estring(char) @@ -1454,7 +1451,7 @@ PACKET_RULESET_UNIT = 140; sc, lsend VLAYER vlayer; - STRVEC helptext[MAX_LEN_PACKET]; + STRVEC helptext; BV_UTYPE_FLAGS flags; BV_UTYPE_ROLES roles; @@ -1515,7 +1512,7 @@ PACKET_RULESET_SPECIALIST = 142; sc, lsend UINT8 reqs_count; REQUIREMENT reqs[MAX_NUM_REQS:reqs_count]; - STRVEC helptext[MAX_LEN_PACKET]; + STRVEC helptext; end PACKET_RULESET_GOVERNMENT_RULER_TITLE = 143; sc, lsend @@ -1537,7 +1534,7 @@ PACKET_RULESET_TECH = 144; sc, lsend UINT32 num_reqs; STRING name[MAX_LEN_NAME]; STRING rule_name[MAX_LEN_NAME]; - STRVEC helptext[MAX_LEN_PACKET]; + STRVEC helptext; STRING graphic_str[MAX_LEN_NAME]; STRING graphic_alt[MAX_LEN_NAME]; end @@ -1565,7 +1562,7 @@ PACKET_RULESET_GOVERNMENT = 145; sc, lsend STRING rule_name[MAX_LEN_NAME]; STRING graphic_str[MAX_LEN_NAME]; STRING graphic_alt[MAX_LEN_NAME]; - STRVEC helptext[MAX_LEN_PACKET]; + STRVEC helptext; end PACKET_RULESET_TERRAIN_CONTROL = 146; sc, lsend @@ -1680,7 +1677,7 @@ PACKET_RULESET_BUILDING = 150; sc, lsend STRING soundtag[MAX_LEN_NAME]; STRING soundtag_alt[MAX_LEN_NAME]; STRING soundtag_alt2[MAX_LEN_NAME]; - STRVEC helptext[MAX_LEN_PACKET]; + STRVEC helptext; end PACKET_RULESET_IMPR_FLAG = 20; sc, lsend @@ -1742,7 +1739,7 @@ PACKET_RULESET_TERRAIN = 151; sc, lsend UINT8 color_green; UINT8 color_blue; - STRVEC helptext[MAX_LEN_PACKET]; + STRVEC helptext; end PACKET_RULESET_TERRAIN_FLAG = 231; sc, lsend @@ -1760,7 +1757,7 @@ PACKET_RULESET_UNIT_CLASS = 152; sc, lsend UINT16 non_native_def_pct; BV_UCLASS_FLAGS flags; - STRVEC helptext[MAX_LEN_PACKET]; + STRVEC helptext; end PACKET_RULESET_EXTRA = 232; sc, lsend @@ -1804,7 +1801,7 @@ PACKET_RULESET_EXTRA = 232; sc, lsend BV_EXTRAS bridged_over; BV_EXTRAS conflicts; SINT8 no_aggr_near_city; - STRVEC helptext[MAX_LEN_PACKET]; + STRVEC helptext; end PACKET_RULESET_EXTRA_FLAG = 226; sc, lsend @@ -1847,7 +1844,7 @@ PACKET_RULESET_GOODS = 248; sc, lsend UINT16 to_pct; UINT16 onetime_pct; BV_GOODS_FLAGS flags; - STRVEC helptext[MAX_LEN_PACKET]; + STRVEC helptext; end PACKET_RULESET_DISASTER = 224; sc, lsend @@ -1917,7 +1914,7 @@ end PACKET_RULESET_COUNTER = 513; sc, lsend STRING name[MAX_LEN_NAME]; STRING rule_name[MAX_LEN_NAME]; - STRVEC helptext[MAX_LEN_PACKET]; + STRVEC helptext; UINT32 def; UINT32 checkpoint; COUNTER_TARGET type; @@ -1945,7 +1942,7 @@ PACKET_RULESET_MULTIPLIER = 243; sc, dsend, lsend STRING rule_name[MAX_LEN_NAME]; UINT8 reqs_count; REQUIREMENT reqs[MAX_NUM_REQS:reqs_count]; - STRVEC helptext[MAX_LEN_PACKET]; + STRVEC helptext; end PACKET_RULESET_CLAUSE = 512; sc, lsend diff --git a/common/networking/packets.h b/common/networking/packets.h index 17c5faac41..5de13f12fc 100644 --- a/common/networking/packets.h +++ b/common/networking/packets.h @@ -181,20 +181,15 @@ int send_packet_data(struct connection *pc, unsigned char *data, int len, enum packet_type packet_type); bool packet_check(struct data_in *din, struct connection *pc); -/* Utilities to exchange strings and string vectors. */ -#define PACKET_STRVEC_SEPARATOR '\3' -#define PACKET_STRVEC_COMPUTE(str, strvec) \ - if (NULL != strvec) { \ - strvec_to_str(strvec, PACKET_STRVEC_SEPARATOR, str, sizeof(str)); \ - } else { \ - str[0] = '\0'; \ - } -#define PACKET_STRVEC_EXTRACT(strvec, str) \ - if ('\0' != str[0]) { \ - strvec = strvec_new(); \ - strvec_from_str(strvec, PACKET_STRVEC_SEPARATOR, str); \ - } else { \ - strvec = NULL; \ +/* Utilities to move string vectors in and out of packets. */ +#define PACKET_STRVEC_INSERT(dest, src) \ + dest = src +#define PACKET_STRVEC_EXTRACT(dest, src) \ + if (src != nullptr && strvec_size(src) > 0) { \ + dest = strvec_new(); \ + strvec_copy(dest, src); \ + } else { \ + dest = nullptr; \ } #ifdef __cplusplus diff --git a/common/networking/packets_json.h b/common/networking/packets_json.h index 1542b20475..677b2ae9eb 100644 --- a/common/networking/packets_json.h +++ b/common/networking/packets_json.h @@ -106,20 +106,15 @@ void *get_packet_from_connection_json(struct connection *pc, FREE_PACKET_STRUCT(&packet_buf); \ return NULL; -/* Utilities to exchange strings and string vectors. */ -#define PACKET_STRVEC_SEPARATOR '\3' -#define PACKET_STRVEC_COMPUTE(str, strvec) \ - if (NULL != strvec) { \ - strvec_to_str(strvec, PACKET_STRVEC_SEPARATOR, str, sizeof(str)); \ - } else { \ - str[0] = '\0'; \ - } -#define PACKET_STRVEC_EXTRACT(strvec, str) \ - if ('\0' != str[0]) { \ - strvec = strvec_new(); \ - strvec_from_str(strvec, PACKET_STRVEC_SEPARATOR, str); \ - } else { \ - strvec = NULL; \ +/* Utilities to move string vectors in and out of packets. */ +#define PACKET_STRVEC_INSERT(dest, src) \ + dest = src +#define PACKET_STRVEC_EXTRACT(dest, src) \ + if (src != nullptr && strvec_size(src) > 0) { \ + dest = strvec_new(); \ + strvec_copy(dest, src); \ + } else { \ + dest = nullptr; \ } #ifdef __cplusplus diff --git a/fc_version b/fc_version index 32b13aa13f..7eed3796f1 100755 --- a/fc_version +++ b/fc_version @@ -77,7 +77,7 @@ DEFAULT_FOLLOW_TAG=S3_3 # On FREECIV_DEBUG builds, optional capability "debug" gets automatically # appended to this. # -NETWORK_CAPSTRING="+Freeciv.Devel-${MAIN_VERSION}-2024.Apr.17" +NETWORK_CAPSTRING="+Freeciv.Devel-${MAIN_VERSION}-2024.Apr.20" FREECIV_DISTRIBUTOR="" if test "x$FREECIV_LABEL_FORCE" != "x" ; then diff --git a/server/ruleset/ruleload.c b/server/ruleset/ruleload.c index 23be2fed18..c5fd51faa3 100644 --- a/server/ruleset/ruleload.c +++ b/server/ruleset/ruleload.c @@ -7921,7 +7921,7 @@ static void send_ruleset_unit_classes(struct conn_list *dest) packet.non_native_def_pct = c->non_native_def_pct; packet.flags = c->flags; - PACKET_STRVEC_COMPUTE(packet.helptext, c->helptext); + PACKET_STRVEC_INSERT(packet.helptext, c->helptext); lsend_packet_ruleset_unit_class(dest, &packet); } unit_class_iterate_end; @@ -8033,7 +8033,7 @@ static void send_ruleset_units(struct conn_list *dest) packet.work_raise_chance[i] = vlevel->work_raise_chance; } } - PACKET_STRVEC_COMPUTE(packet.helptext, u->helptext); + PACKET_STRVEC_INSERT(packet.helptext, u->helptext); packet.worker = u->adv.worker; @@ -8090,7 +8090,7 @@ static void send_ruleset_specialists(struct conn_list *dest) } requirement_vector_iterate_end; packet.reqs_count = j; - PACKET_STRVEC_COMPUTE(packet.helptext, s->helptext); + PACKET_STRVEC_INSERT(packet.helptext, s->helptext); lsend_packet_ruleset_specialist(dest, &packet); } specialist_type_iterate_end; @@ -8198,7 +8198,7 @@ static void send_ruleset_techs(struct conn_list *dest) packet.flags = a->flags; packet.cost = a->cost; packet.num_reqs = a->num_reqs; - PACKET_STRVEC_COMPUTE(packet.helptext, a->helptext); + PACKET_STRVEC_INSERT(packet.helptext, a->helptext); lsend_packet_ruleset_tech(dest, &packet); } advance_iterate_end; @@ -8219,7 +8219,7 @@ static void send_ruleset_counters(struct conn_list *dest) packet.type = pcount->target; packet.def = pcount->def; - PACKET_STRVEC_COMPUTE(packet.helptext, pcount->helptext); + PACKET_STRVEC_INSERT(packet.helptext, pcount->helptext); lsend_packet_ruleset_counter(dest, &packet); } city_counters_iterate_end; } @@ -8284,7 +8284,7 @@ static void send_ruleset_buildings(struct conn_list *dest) sz_strlcpy(packet.soundtag, b->soundtag); sz_strlcpy(packet.soundtag_alt, b->soundtag_alt); sz_strlcpy(packet.soundtag_alt2, b->soundtag_alt2); - PACKET_STRVEC_COMPUTE(packet.helptext, b->helptext); + PACKET_STRVEC_INSERT(packet.helptext, b->helptext); lsend_packet_ruleset_building(dest, &packet); } improvement_iterate_end; @@ -8392,7 +8392,7 @@ static void send_ruleset_terrain(struct conn_list *dest) packet.color_green = pterrain->rgb->g; packet.color_blue = pterrain->rgb->b; - PACKET_STRVEC_COMPUTE(packet.helptext, pterrain->helptext); + PACKET_STRVEC_INSERT(packet.helptext, pterrain->helptext); lsend_packet_ruleset_terrain(dest, &packet); } terrain_type_iterate_end; @@ -8526,7 +8526,7 @@ static void send_ruleset_extras(struct conn_list *dest) packet.bridged_over = e->bridged_over; packet.conflicts = e->conflicts; - PACKET_STRVEC_COMPUTE(packet.helptext, e->helptext); + PACKET_STRVEC_INSERT(packet.helptext, e->helptext); lsend_packet_ruleset_extra(dest, &packet); } extra_type_iterate_end; @@ -8620,7 +8620,7 @@ static void send_ruleset_goods(struct conn_list *dest) packet.onetime_pct = g->onetime_pct; packet.flags = g->flags; - PACKET_STRVEC_COMPUTE(packet.helptext, g->helptext); + PACKET_STRVEC_INSERT(packet.helptext, g->helptext); lsend_packet_ruleset_goods(dest, &packet); } goods_type_iterate_end; @@ -8816,7 +8816,7 @@ static void send_ruleset_governments(struct conn_list *dest) sz_strlcpy(gov.rule_name, rule_name_get(&g->name)); sz_strlcpy(gov.graphic_str, g->graphic_str); sz_strlcpy(gov.graphic_alt, g->graphic_alt); - PACKET_STRVEC_COMPUTE(gov.helptext, g->helptext); + PACKET_STRVEC_INSERT(gov.helptext, g->helptext); lsend_packet_ruleset_government(dest, &gov); @@ -9027,7 +9027,7 @@ static void send_ruleset_multipliers(struct conn_list *dest) } requirement_vector_iterate_end; packet.reqs_count = j; - PACKET_STRVEC_COMPUTE(packet.helptext, pmul->helptext); + PACKET_STRVEC_INSERT(packet.helptext, pmul->helptext); lsend_packet_ruleset_multiplier(dest, &packet); } multipliers_iterate_end; -- 2.34.1