[Freeciv-Dev] Re: Generator script

[Raimar Falke <rf13@xxxxxxxxxxxxxxxxx>]

On Mon, Sep 09, 2002 at 12:28:14AM +0200, Peter Hans van den Muijzenberg wrote:
> hi,
> 
> On 05-sep-02 Raimar Falke wrote:
> 
> 
> RF> On Mon, Sep 02, 2002 at 05:23:32PM +0200, Peter Hans van den Muijzenberg
> RF> wrote:
> PHM>> On 28-aug-02 Raimar Falke wrote: 
> RF>>> On Wed, Aug 28, 2002 at 07:27:00PM +0200, Peter Hans van den
> RF>>> Muijzenberg wrote:
> 
> 
> PHM>> So far, I did the file parsing. It operates the simple way: You add a
> PHM>> marker (atm the comment 'GENERATE') before any structure you want to
> be
> PHM>> sendable, and the script gets the structure out. Only other difference
> PHM>> in my test file, packets.h, is that I split off nameless, embedded
> PHM>> structures. I'd rather not go there in a simple script. Would this be
> a
> PHM>> problem? (I don't know the design decision that created them to begin
> PHM>> with.) I believe I could see one more problem coming, but I'm not sure
> PHM>> now:
> RF> IMHO: before we create such a script we should "normalize" (an
> RF> overloaded term these days) the packets. There are irregularities
> RF> like:
> 
> RF>  if(unit_type_flag(packet->id, F_PARATROOPERS)) {
> RF>    cptr=put_uint16(cptr, packet->paratroopers_range);
> RF>    cptr=put_uint8(cptr, packet->paratroopers_mr_req);
> RF>    cptr=put_uint8(cptr, packet->paratroopers_mr_sub);
> RF>  }
> 
> RF> or the handling of the helptexts.
> 
> Agreed. having looked into it in more detail now that the script generates
> send functions, I found that the relation between the structures and their
> send-functions isn't as straight forward as I'd assumed, apparently from
> meeting the straight ones first. The history of the project appears to have
> dumped quite a lot of stuff in packets that doesn't have to do with /how/
> te information is represented on the network, but with /what/ information
> is to be represented.

> The case of the help texts can be approached be typedef char *helptext; and
> by forcing a field of type helptext to the end of the structure or packet,
> I think. It doesn't seem to be a real part of either. It's more like the
> connection allows having each packet be accompanied by a message.

The solution is IMHO to send the empty string and just take the extra
byte which the current construct avoids.

> RF> You also have to tag the ints in the
> RF> struct packet_* so that the script knows if it needs to send a 1, 2 or
> RF> 4 byte value.
> 
> And signed or unsigned, yes. All the required distinction and merger of
> types was what i referred to as "some issues" that existed. I've taken the
> easy way and used for my packets.h, in those places where I thought I knew
> what to pick, typedefs for uint8, uint16, uint32, and sint16. Of course,
> naming these types according to their use would be more helpful for the
> rest of the code, but that requires someone who knows that rest. (-: I
> can't say what eg. the typical type for an ID, a coordinate, a laps of
> time, or an amount of money would be.

> (Some of those appear to use different sizes at different times.)

This was to be expected.

> But in all, as it is a number of packets would have to take the escape of
> having their current functions pre-specified, which will stop the
> generation of new functions, as there would at the least be
> incompatibilities if things were made simpler.
> (What is the incompatibility-mechansim you're using, BTW?)

This isn't a problem. In the first step we make the packets "simpler"
using "traditional" code. This will change the wire protocol but this
isn't a problem because we have capabilities. In the second step we
can change to code to be generated by a script.

> PHM>>  Is there one function that handles a packet named differently?
> RF> I don't understand.
> 
> The answer is: "Yes, there is a function that uses a name for the packet
> that differs from the name the struct has":
> "int send_packet_ruleset_terrain_control(struct connection *pc, 
>             const struct terrain_misc *packet)"
> uses the name "terrain_control" where the structure has "terrain_misc".
> Which one to macro/typedef? (Small problem compared to the other stuff.)
> 
> 
> 
> [Place of code]
> 
> PHM>> Every send_packet-function, apart from converting, appears to do the
> PHM>> same three things:
> 
> PHM>> - Create a dynamic buffer;
> RF> It doesn't use malloc but allocates it on the stack which is very cheap.
> 
> Well, not necessarily, dependingon the compiler, but basically you're right.
> I must have been doing two things at the same time, as I had ended up with
> the impression they were (fc_)malloc-ed.
> 
> PHM>> - Write the well-formed packet-length into it; - Pass the
> PHM>> packet-length and the buffer to the send_packet_data routine.
> 
> PHM>> Is there a specific reason for not having:
> 
> 
> PHM>> - One static buffer for all (outgoing?) packets;
> RF> It is possible that a send_packet_* function calls another one. AFAIK
> RF> there is no such case at the moment. send_attribute_block comes
> RF> closest to this.
> 
> OK. BTW, would it be better if in such a case the send_packet_a would only
> call a put_packet_b?
> I could imagine the send_packet_a(...) being
> { 
>   SEND_PACKET_START( ...)
> 
>   put_packet_a(...);
> 
>   SEND_PACKET_END(...)
> }
> with the actual putting being done in the put_packet function. This would
> also allow the lsend_packet function to put_packet only once, and send
> repeatedly. The cost is the extra call for single send-s. (I don't know
> whether the ratio between the one and the other makes this worth
> implementing. It is a bit cleaner, though.)

This would double the number of functions. And packets.c is already
large. The performance gain is IMHO very small. But I agree that it is
cleaner.

> RF> It is cleaner this way. Don't use globals.
> 
> Well, I sometimes do, as the property of a specific module, like packets
> here, though usually the module ends up as an abstract data structure and
> the global as one of the structure's fields. (-: OK. 
> 
> 
> PHM>> - The send-packet function merely /passing/ /in/ the packet-length in
> PHM>> send_packet_data;
> 
> PHM>> - The send_packet_data routine adding the well-formed packet-length to
> PHM>> the packet?
> 
> RF> Yes this is quite possible. And with the recent invention of
> RF> SEND_PACKET_* it should be a small patch.
> 
> Oops, I'm doing this from the public version. I now checked the CVS-view.
> Yes, it would. having looked into this a bit more, I feel that design-wise
> this shouldn't be in packets at all. It's the concern of connection that
> packets are sent accross the network preceeded by their length; if someone
> were to switch to a system allowing packets of 70.000 bytes long, then
> packets should not have to change because of that. But as I'm currently
> merely familiarizing myself with the code a bit by writing a script that
> processes some of it, I'll stick to the idea of moving the storing of the
> length one level. (-: 

This is a philosophical point.

> PHM>>>> BTW, do you know that the current implementation of packets.c is
> PHM>>>> not, as the documentation has it "highly portable" C-code. You'd
> PHM>>>> never notice as freeciv already requires IX-like for other
> PHM>>>> issues, but the PHM>> conversion calls are GNU-specific, or possibly
> PHM>>>> IX-specific.
> 
> RF> According to my man-page htonl, htons, ntohl, ntohs are from BSD 4.3.
> 
> RF> It looks to me that this is rather a libc than a compiler issue.
> 
> Well, the library of standard C-function varies from compiler to compiler.
> Only those defined in the C standards can be expected to be present with
> all.

> RF> Nevertheless reimplementing these functions is easy.
> 
> Sure. Does that mean they ought to be added to support, so the code becomes
> more portable?

Only if you show me a platform which doesn't have them.

> RF> But IMHO we can expect to find them under this name of similar on
> RF> every system because they are needed for TCP/IP.
> 
> As I said, you'd probably never notice, but the guy who tries to implement
> freeciv on a different networking protocol might be somewhat surprised. (-:
> 
> 
> 
> PHM>> Similarly, I'm not sure FreeCiv can guarantee that an int will
> PHM>> actually hold 32 bit (or more).
> RF> Freeciv runs on platforms where int is 64bit. But it will break on
> RF> sizeof(int)<4 and also if sizeof(int)!=sizeof(enum). And maybe some
> RF> others.
> 
> It might be worth mentioning these limitations to the "highly portable"
> somewhere. For the developers, since it tells them what they can assume to
> be present. And for the porters, as it tells them what is required. That's
> more or less what I was steering towards.

Well nobody has yet met such a platform. I expect that such platforms
exists but they are rather exotic.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
  "Real Users find the one combination of bizarre
   input values that shuts down the system for days."