Yuichiro NAITO wrote:> Take a look at my GitHub pull request to see my patch. > > https://github.com/openssh/openssh-portable/pull/216I think the length at the beginning should be tied to the (number of?) members that are sent instead of the struct passwd size on either side. Also, adding passwd.fields seems to be an unrelated change, better placed in a separate commit. Finally, sshbuf_free_passwd() is added but never called. If it is not needed then I think it's better to not add it (yet). //Peter
Thanks for reviewing my patch.> 2020/11/20 23:45?Peter Stuge <peter at stuge.se>????: > > Yuichiro NAITO wrote: >> Take a look at my GitHub pull request to see my patch. >> >> https://github.com/openssh/openssh-portable/pull/216 > > I think the length at the beginning should be tied to the (number of?) > members that are sent instead of the struct passwd size on either side.OK. I fixed to send number of struct passwd members at first in sshbuf_put_passwd(). And sshbuf_get_passwd() checks it.> Also, adding passwd.fields seems to be an unrelated change, better > placed in a separate commit.It is used for FreeBSD sturct passwd. I made a different commit and wrote about this in the message. So I force pushed new branch to the same Pull Request. Please refer to the new one.> Finally, sshbuf_free_passwd() is added but never called. If it is not > needed then I think it's better to not add it (yet).sshbuf_free_passwd() is used for error case of sshbuf_get_passwd(). I think it is easy to read and worth to be remained. ? Yuichiro NAITO naito.yuichiro at gmail.com