On 04/28/2011 02:47 PM, Kurt Van Dijck wrote:
>> I quickly looked over the code, see some comments inline.
>>
> Thanks, I have some additional questions:
>>> ---
>>> + /* run */
>>> + while (1) {
>>> + ret = poll(pfd, 2, -1);
>> ARRAY_SIZE?
> is that available in userspace? cool!
...if you define it :)
>>> + if (ret < 0)
>>> + error(1, errno, "poll()");
>>
>> what about EINTR?
>>
> well, that's indeed a typical one, but the remainder of the program is
> not interested in any signal...
If you attach strace to this program while waiting in poll it will exit.
>>> + break;
>>> + }
>>> + len = ret;
>>> + for (done = 0; done < len; ) {
>>> + ret = send(pfd[1].fd, buf, len, s.sendflags);
>>> + if (ret < 0) {
>>> + error(0, errno, "write(%s)",
>>> libj1939_addr2str(&s.src));
>>> + if (ENOBUFS == errno) {
>>
>> the other way round?
>>
> ???
just nitpicking:
In the "kernel" we usually compare like this:
if ($VARIABLE == $CONSTANT)
not ($CONSTANT == $VARIABLE)
>>> + sleep(1);
>>> + continue;
>>> + }
>>> + exit(1);
>>> + }
>>> + done += ret;
>>> + }
>>> + }
>>> + if (pfd[1].revents) {
>>> + ret = read(pfd[1].fd, buf, sizeof(buf));
>>> + if (ret < 0) {
>>> + ret = errno;
>>> + error(0, errno, "read(%s)",
>>> libj1939_addr2str(&s.dst));
>>> + switch (ret) {
>>> + case EHOSTDOWN:
>>> + break;
>>> + default:
>>> + exit(1);
>>> + }
>>> + } else {
>>> + write(STDOUT_FILENO, buf, ret);
>>> + }
>>> + }
>>> + }
>>> +
>>> + free(buf);
>> close()?
> will close 1 statement further...
>>> + return 0;
>>> +}
> closed!
> is that good enough?
yep - you might remove that free then, too :P
>
>>> +//-----------------------------------------------------------------------------
>>> +
>>> +//-----------------------------------------------------------------------------
>>> +static struct ifname *libj1939_add_ifnam(int ifindex, const char *str)
>>> +{
>>> + struct ifname *nam;
>>> + nam = malloc(sizeof(*nam) + strlen(str));
>>> + memset(nam, 0, sizeof(*nam));
>> calloc?
> well, that's possible. I tend to not use calloc, but that's personal.
> In fact, you're right here.
Ohh - haven't even seen this :)
>>> + nam->ifindex = ifindex;
>>> + strcpy(nam->name, str);
>>> + nam->next = s.names;
>>> + s.names = nam;
>>> + return nam;
>>> +}
>>> +
>
> Kurt
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Socketcan-core mailing list [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-core
