Follow-up Comment #7, patch #1296 (project freeciv):

>> if (ptile) {
>> /* Is it useful? */
>> packet->x = ptile->x;
>> packet->y = ptile->y;
>> }
>
>
> I think the reset of the coordinates (x,y) within the packet is
> needed, as the same packet struct could also be used for offline
> events.

Changed this comment to 'Set the coordinates back, to make the packet still
usable.'

>> Why is this not simple strncpy()?
>
>
> I do most of my coding as learning-by-doing. Sorry if I did
> not choose the right function or if there are bad code
> sections. Here I did select the first function which did that
> I want. You did delete the block completely and I think its a
> better solution.

I do the same.  I am working on Freeciv code for 3 years, and I still ignore
some things.

I am sorry if sometimes, the comments looks a bit too affirmative, but
actually, there are advices.  Don't worry if people comment your patch, it
marks at least they have interest on it.

Actually the usage of strncpy() is not the best in my opinion in this case. 
sz_strlcpy() (defined in utility/support.h) alias auto-sized mystrlcpy()
should be faster because it stops when the string ends and unsure it finish
by a nul character.

In your previous patch, you should also have used only: 'packet_send =
*packet;', then all fields, including the string would have been copied.


(file #6829)
    _______________________________________________________

Additional Item Attachment:

File name: trunk_S2_2_restructure_notify_conn2.diff Size:7 KB


    _______________________________________________________

Reply to this item at:

  <http://gna.org/patch/?1296>

_______________________________________________
  Message posté via/par Gna!
  http://gna.org/


_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

Reply via email to