Re: libssh2 master 37624b6 sftp_attrsize: converted function to a macro

On Wed, 28 Apr 2010, Peter Stuge wrote:

>> sftp->open_packet_len = filename_len + 13 +
>> - (open_file? (4 + sftp_attrsize(&attrs)) : 0);
>> + (open_file? (4 + sftp_attrsize(LIBSSH2_SFTP_ATTR_PERMISSIONS)) : 0);
>
> Where does attrs come from? If it's not created in the function this
> might desync libssh2 and server expectations.

It's a variable declared just a few lines above this code.

> Same for other occurences of sftp_attrsize() on constants, where the return
> value is used in a packet. It just seems more defensive to me, and less
> redundant, to use sftp_attrsize() on the actual value in the packet.

Well of course. sftp_attrsize() is used at four places, two of them with
actual incoming data and two times with a fixed compile-time value.

>> + /* surprise! this starts out with nothing sent */
>> + sftp->open_packet_sent = 0;
>
> This looks redundant. I think there may be some more places where
> there's code like this - it'd be nice to reduce rather than expand. :)

Redundant how? If you can write the code without that or similar logic, then
please explain.

-- 
  / daniel.haxx.se
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel