Re: CVS commit: src/sys
[like pros, we were talking on source-changes list. that wasn't very useful] On 02/12/14 04:28, Ryota Ozaki wrote: On Tue, Dec 2, 2014 at 12:39 PM, Antti Kantee po...@iki.fi wrote: On 02/12/14 03:30, Ryota Ozaki wrote: The problem is that the real if_drain_all() will never be called in environments with the weak alias problem even if the non-stub implementation is present and it should be called. I'm getting the problem, but I have still one question; why ifnet_list works while if_drain_all doesn't? If you are asking about the linker, ifnet_list is a common symbol, not a weak alias. If you are asking about traversing ifnet_list, semi-accidentally -- the tailq macros happen to work for zero-filled heads unless you try to tailq_insert_tail. I meant the former. Anyway I got it. rumpnet doesn't add any items to ifnet_list, so it works without rumpnet_net. Right, because interfaces don't exist in net, only in net_net (it makes sense if you think about sys vs. sys/net). Actually, looking more closely, it is currently a problem for _all_ platforms since you stubbed it to a panic instead of a nop. Oh, right. I just followed bridge ones... If you look at the history of that file, you will notice that I have been removing __weak_alias() instances. bridge/agr not working on platforms without weak alias support is acceptable (because it never has worked). I hope bridge/agr can be fixed some day, but perhaps they will fix themselves with a more modular networking stack ;) We can get rid of __weak_alias(bridge_input) already :) The other two are...I don't know when it's done :-/ Oh, nice! Can you remove the now-unnecessary weak aliases?
Re: CVS commit: src/sys
On Tue, Dec 2, 2014 at 8:25 PM, Antti Kantee po...@iki.fi wrote: [like pros, we were talking on source-changes list. that wasn't very useful] On 02/12/14 04:28, Ryota Ozaki wrote: On Tue, Dec 2, 2014 at 12:39 PM, Antti Kantee po...@iki.fi wrote: On 02/12/14 03:30, Ryota Ozaki wrote: The problem is that the real if_drain_all() will never be called in environments with the weak alias problem even if the non-stub implementation is present and it should be called. I'm getting the problem, but I have still one question; why ifnet_list works while if_drain_all doesn't? If you are asking about the linker, ifnet_list is a common symbol, not a weak alias. If you are asking about traversing ifnet_list, semi-accidentally -- the tailq macros happen to work for zero-filled heads unless you try to tailq_insert_tail. I meant the former. Anyway I got it. rumpnet doesn't add any items to ifnet_list, so it works without rumpnet_net. Right, because interfaces don't exist in net, only in net_net (it makes sense if you think about sys vs. sys/net). Actually, looking more closely, it is currently a problem for _all_ platforms since you stubbed it to a panic instead of a nop. Oh, right. I just followed bridge ones... If you look at the history of that file, you will notice that I have been removing __weak_alias() instances. bridge/agr not working on platforms without weak alias support is acceptable (because it never has worked). I hope bridge/agr can be fixed some day, but perhaps they will fix themselves with a more modular networking stack ;) We can get rid of __weak_alias(bridge_input) already :) The other two are...I don't know when it's done :-/ Oh, nice! Can you remove the now-unnecessary weak aliases? Done. I'm thinking how to remove weak alias for bridge_ifdetach. It can be done by having a (if)detach callback list that callbacks are called in ether_ifdetach. We can register bpf_detach and vlan_ifdetach to the list as well as bridge_ifdetach. I don't know it's worthwhile to do though. ozaki-r
Re: CVS commit: src/sys/arch/atari/stand/installboot
On Fri, Nov 14, 2014 at 02:43:39AM +0900, Izumi Tsutsui wrote: christos@ wrote: Module Name:src Committed By: christos Date: Thu Nov 13 17:19:29 UTC 2014 Modified Files: src/sys/arch/atari/stand/installboot: installboot.c Log Message: fix strict aliasing violations - *((u_int16_t *)bb-bb_xxboot + 255) = 0; - *((u_int16_t *)bb-bb_xxboot + 255) = 0x1234 - abcksum(bb-bb_xxboot); + sum = 0; + memcpy(bb-bb_xxboot + 255, sum, sizeof(sum)); + sum = 0x1234 - abcksum(bb-bb_xxboot); + memcpy(bb-bb_xxboot + 255, sum, sizeof(sum)); I doubt your bb-bb_xxboot + 255 is the same place as the original (u_int16_t *)bb-bb_xxboot + 255 (the cksum word looks at the end of 512 byte sector). memcpy(9) looks also awful for readers because it hides endianness.. (Having read all the thread...) I'm pretty sure you can cast between a pointer to a union and a pointer to one of its mmembers. So under the assumption that the compiler will let you alias between members of a union (if it doesn't then far more stuff will break) the following is safe: Define a union that contains a 'bb' and a u_int16_t []. Declare a variable that is a pointer to the union. Assign (with cast) the address of 'bb' to the pointer variable. Access the other member of the union. The only other alternative is to do all the accesses as 'char'. I'd guess that if the compiler inlines memcpy() it can 'know' about the types of the variables involved - and apply the 'strict aliasing' rules in the same way that is applied the 'alignment' ones. David -- David Laight: da...@l8s.co.uk
Re: CVS commit: src/sys/arch/atari/stand/installboot
On Mon, Nov 24, 2014 at 02:36:51PM +, Taylor R Campbell wrote: In the case of abcksum, if you have uint8_t p[512]; ... *((uint16_t *)p + 255) = 0; *((uint16_t *)p + 255) = abcksum(p); GCC may choose to evaluate abcksum before the zero assignment, because no uint16_t is allowed to alias a uint8_t, so the assignment of an lvalue with type uint16_t can't change the value of abcksum(p). Not sure, the accesses of p[] inside abcksum() are allowed to alias any other memory - this includes wherever was written to by the *((uint16_t *)xxx) = 0; regardless of any expected knowledge of the value of xxx. For gcc you can add asm volatile(:::memory); to force it to 'forget' anything it thinks it knows about the contents of memory. The original checksum code can be (mostly) fixed by only having a single (uint16_t) cast. You are then only left with problems caused if the checksum is inlined and any earlier writes to the sector itself are still 'pending'. David -- David Laight: da...@l8s.co.uk
re: CVS commit: src/usr.bin/w
Christos Zoulas writes: Module Name: src Committed By: christos Date: Tue Dec 2 22:19:19 UTC 2014 Modified Files: src/usr.bin/w: w.c Log Message: if doing uptime, don't bother resolving names. this helps, but isn't it just hiding the fact that uptime is doing stuff it does not need to? ie, shouldn't we find what that is and stop it happening? now (as before) uptime is doing more work than it should.. .mrg.
