**** BEGIN LOGGING AT Fri Jan 28 02:59:56 2011 Jan 28 12:02:21 holtmann: ping Jan 28 12:02:36 pong Jan 28 12:03:31 holtmann: I would prefer to use g_try_new0 instead of g_new0 as later will abort the program Jan 28 12:04:01 Let me put it this way. If you can't allocate 10 bytes of memory anymore, you do have other problems in your system. Jan 28 12:04:27 Also the Linux kernel will start killing other processes before you do not get your memory. Jan 28 12:05:27 So in general I agree. memory allocation NULL checks are good. However we need to figure out a healthy relation between readable code, real case of just abort is fine and going overboard with this. Jan 28 12:06:40 hmm Jan 28 12:07:27 Think about it for a bit. Are you sure we will ever fail for 10 bytes of memory. Jan 28 12:08:06 Btw. there is clearly a bug here. Using g_try_new0 without check is wrong. I would just go for g_new0 in this case. Jan 28 12:08:24 Or we go for check with a goto. Jan 28 12:08:51 But not NULL check && g_at_chat_send in the same if statement. Since that goes out of control in the long run. Jan 28 12:11:20 ok Jan 28 12:12:08 Jeevaka: And btw. I am totally old school with memory allocation. My brain is trained to add checks here, but I think we have to face reality that the world moved on. Jan 28 12:13:25 Same as that libc free() and g_free() do the NULL check for you. Jan 28 12:13:41 can we follow the same throughout the ofono Jan 28 12:14:19 if we are planning to use g_new0 which to me is also ok, then we should use only g_new0 throughout ofono Jan 28 12:15:03 code will be free from plenty of ifs and buts and also goto error Jan 28 12:15:41 Now you are raising a funny question that Denis and I have been debating for a while now. Jan 28 12:16:25 So in theory for all small struct declaration we should go with g_new0 and only do large memory allocation with g_try_new0 (like ringbuffers etc). Jan 28 12:16:37 So where is a good threshold? Jan 28 12:16:41 I have no answer for that. Jan 28 12:17:06 consider the situation where we are registered to network, for one byte allocation failure aborting the program seems not good Jan 28 12:17:28 Agreed, but the kernel will not abort you if you just ask for one byte. Jan 28 12:17:31 yes, at some point it becomes useful to check for allocation errors Jan 28 12:17:49 There is always memory available that you can get. Jan 28 12:17:59 for eg: if I want to know the SS status info, its ok to fail the request instead of aborting the program Jan 28 12:18:19 I see your point, but what does OOM in Linux really means. Jan 28 12:18:55 Jeevaka: most likely you won't be able to return the error over dbus beause that alone requires allocating some bytes Jan 28 12:18:57 The kernel already has the concept of atomic and non-atomic allocation. It manages a reserve of always guaranteed memory. Jan 28 12:19:19 If that reserve fails, you are in deep trouble with your system. It is going to malfunction on so many level, you have no idea. Jan 28 12:20:13 If you ask for 1k for a buffer, then yes g_try_new0 is the right thing to do, but for simple structs it is just pointless. Since it makes no difference. You will always get your memory. Jan 28 12:20:28 see the point Jan 28 12:20:36 Jeevaka: So here is the fun thing with this. In a lot of cases it is so simple to check for memory that you just add the check. Jan 28 12:20:53 And then you have cases like the one you just pointed out where it cripples the code. Jan 28 12:21:20 actually, I thought of going for cbd == NULL check first Jan 28 12:21:33 I am almost for going just accepting the program abort here. Since lets face it, it will never ever happen anyway. Jan 28 12:22:02 since it introduces more number of lines, i decided to go ahead using with g_at_chat_send Jan 28 12:22:26 I figured, but it does not make this more readable, does it? Jan 28 12:23:08 agree, readability has been sacrified a little Jan 28 12:23:13 :) Jan 28 12:23:51 since cb_data_new is a generic function, I'll go ahead and modify the patch with the only NULL check, is it ok? Jan 28 12:24:34 Let's wait for Denis. As I said, we have been discussing this forth and back a few times already. Jan 28 12:24:44 ok Jan 28 12:25:02 need your view on one more thing Jan 28 12:25:05 We might need to stick our heads together at some point and figure out a general rule forward for this. Jan 28 12:25:21 This also affects ConnMan and BlueZ as well. Jan 28 12:26:32 all the ./drivers/******/radio-settings.c are freeing the cbd after calling CALLBACK_WITH_FAILURE Jan 28 12:27:35 it would be better to have the free before CALLBACK_WITH_FAILURE. Jan 28 12:28:15 Why? Jan 28 12:29:42 nothing wrong. but it would be nice to free when we dont need it any more Jan 28 12:30:18 I don't really think that this is a big deal. Some of them reference cbd->data still and there you have no choice. Jan 28 12:31:59 ok Jan 28 12:39:05 holtmann, denkenz, padovan: Do you know if there is a way to be notified when a modem is powered ON? I have seen in modem.c that there is online_watch but I need a sort of powered_watch on each modem to create the AT emulator. Jan 28 13:01:16 Gzajac: pre_sim method in driver? Jan 28 13:04:27 pessi: yes, I could use also enable method also, however the emulator has to be independant from the modem drivers. Jan 28 13:05:48 you could load it like any atom Jan 28 13:05:59 or like message-waiting atom Jan 28 13:07:14 or add __ofono_emulator_probe_drivers(modem) in modem_change_state()? Jan 28 13:14:10 the problem is emulator atom is not like voice-call, message-waiting, ... services Jan 28 13:14:10 it can't be created through modem drivers. Jan 28 13:14:10 it should be created using a plugin like smart-messaging one. Jan 28 13:15:22 usgin watcher on each modem. Jan 28 13:15:27 really? Jan 28 13:18:27 yes it would be the cleanest way Jan 28 13:19:15 the adding __ofono_emulator_probe_drivers(modem) in modem.c is not very clean, there would be some code very specific to emulator into modem code Jan 28 13:21:03 Denis advised me not to create emulator atom into modem drivers. Jan 28 14:58:44 Hi Jan 28 15:00:08 I try to run ofono-0.39 on my host connected to a Sagem modem (dev kit) on ttyUSB0 but I have no modem listed with the test script list-modem Jan 28 15:01:52 I have tried with ofono-0.21 last year and work Jan 28 15:03:42 is there a tutorial howto start with ofono ? Jan 28 15:04:01 Jeevaka: So my rule of thumb for g_new would be something like: If the allocated memory is less than a typical memory page, don't even bother checking the return Jan 28 15:05:51 YoYo: You need a udev rule. modem.conf is gone. See changelog and announcement. Jan 28 15:06:04 Calypso example should be in the doc/ directory. Jan 28 15:09:08 denkenz: hmm, then in that case patch i sent can be ignored Jan 28 15:10:17 denkenz: do you want me to send a new patch removing the NULL check for memories allocated less than a memory page Jan 28 15:12:17 Jeevaka: Don't go overboard. You will find a lot of cases. Jan 28 15:12:24 However for cb_data we might just remove them. Jan 28 15:12:43 And use g_new0 of course. Jan 28 15:15:37 denkenz: For STK, the (meaningful) statement is to skip the event 'idle screen available'. But, I think, we still need to figure out what is the normal stand by display. Jan 28 15:15:55 For Display Text, there is indeed a GCF test related to the priority. Actually, this priority is handled within the STK agent. Jan 28 15:16:41 It means that the STK agent needs to distinguish the state where the device is busy and can't display a normal priority message. Jan 28 15:17:44 thanks I will have a look Jan 28 15:20:01 pnunes: We don't necessarily have to do it this way Jan 28 15:20:28 just always displaying normal messages on the homescreen and high priority going to an always-on-top popup is a possibility Jan 28 15:22:30 I see. Sorry but, the test is precisely asking to not display the message...But again, this test condition can be considered differently regarding our platform capabilities. Jan 28 15:23:37 Test case #? Jan 28 15:24:24 Perhaps, the GCF lab can tolerate such behavior. Jan 28 15:27:08 test case is 27.22.4.1.1 Sequence 1.2 (DISPLAY TEXT normal priority, Unpacked 8 bit data for Text String, screen busy) Jan 28 15:30:22 so basically you're referring to: "The terminal shall reject normal priority text commands if the screen is currently being used for more than its normal stand-by display." Jan 28 15:30:31 For us that is like always Jan 28 15:32:19 denkenz: do you know if we have any modem watch to check whan a modem is powered ON to create AT emulator? Jan 28 15:33:14 if not I have implemented into modem.c, a add_powered_watch() like add_online_watch() Jan 28 15:33:44 to notify the AT emulator plugin when the modem state is powered ON Jan 28 15:34:40 So the test condition "Set the Terminal screen to a display mode other than the normal stand-by display" is simply not possible, therefore the test is not relevant. So, it can be indeed a possibility to skip this test. Jan 28 15:35:32 Gzajac: There's nothing to know that a modem has been powered right now Jan 28 15:35:56 However, which emulator are you specifically interested in? Jan 28 15:36:16 HFP can simply track the presence of voicecall and netreg Jan 28 15:36:37 DUN can track the presence of gprs Jan 28 15:36:55 denkenz: I mean AT emulator, for Dial Up Networking Jan 28 15:38:07 So you strictly don't need it, but I can see that adding a powered watch would make things easier Jan 28 15:38:10 so feel free to add this Jan 28 15:38:17 ok Jan 28 15:39:18 As I wanted to use same idea as smart messaging to create AT emulator. I would need such watcher. Jan 28 15:40:01 holtmann, padovan, denkenz: did you take a look at "add bluetooth server support" patch ? is there any problems ? Jan 28 15:42:06 how add a rule for Udev for my Sagem modem ? Jan 28 15:56:50 anyone help for try ofono ? Jan 28 15:57:22 denkenz: about HFP GW, do you think that all AT commands should be managed by the AT Emulator, or the Bluetooth specific ones (like AT+BLDN for example) should be managed by the plugins ? Jan 28 15:59:47 fdanis: Not sure yet, we still have to figure out how to register command handlers Jan 28 16:00:08 however, BLDN and ATD should probably be handled in the same place Jan 28 16:02:58 denkenz: my first thought was that commands shared by HFP GW and DUN GW should be managed by the AT Emulator (which should register handlers for them) Jan 28 16:05:10 so my thinking was as follows Jan 28 16:05:16 at emulator atom comes online Jan 28 16:05:27 then all specific atoms register handlers to it Jan 28 16:05:34 so e.g. voicecall would register atd, bldn, etc Jan 28 16:05:49 netreg would register those CIEV thingies Jan 28 16:06:26 some external plugin might register battery status Jan 28 16:06:29 etc Jan 28 16:07:21 emulator might just have to provide some apis for common stuff, like CIEV Jan 28 16:11:54 how add a udev rule for use the atmodem driver (modem on ttyUSB0) ? Jan 28 16:17:00 denkenz: I am not sure to understand, this will implies changes in current atoms (voicecall, netreg, ...) to be linked to emulator atom, right ? Jan 28 16:17:11 yep Jan 28 16:17:34 however, that is easier than giving access to the internal atom datastructures Jan 28 16:22:45 so, the AT commands will be managed by the atom in charge of the related capability Jan 28 16:22:45 but how this will be managed for command shared by 2 atoms, like ATD which can be used to perform a voice call or a data call ? Jan 28 16:24:24 we don't support data calls Jan 28 16:24:45 if you mean the gprs variation, lets cross that bridge when we come to it Jan 28 16:25:03 right now we just want hfp and dun basic profiles working Jan 28 16:28:34 what did you mean by "dun basic" ? Jan 28 16:28:34 ATD is one of the basic commands, no ? Jan 28 16:29:07 we have the hfp emulator and we have the dun emulator Jan 28 16:29:10 two separate atoms Jan 28 16:29:18 the atd does not clash Jan 28 16:30:18 when we decide to implement generic modem emulation, then we'll have to figure out how to handle atd##; and atd*99 variations Jan 28 16:30:37 But for now we don't worry about it Jan 28 16:31:04 ok, the emulator is not common for hfp and dun Jan 28 16:48:25 denkenz: ping Jan 28 16:49:31 Jeevaka: pong Jan 28 16:51:03 is there any bug in ste_radio_settings_probe Jan 28 16:53:23 particularly with the GAtChat Jan 28 17:05:05 I dunno Jan 28 17:05:09 you tell me ;) Jan 28 17:06:03 there is Jan 28 17:09:49 you mean its not cloning the GAtChat? Jan 28 17:09:51 then yes that is a bug Jan 28 17:10:37 about the register Jan 28 17:10:57 gatchat cloning also Jan 28 17:12:37 yes, both are bugs Jan 28 17:38:03 denkenz: just to be sure, you received the v3, right? Jan 28 17:42:32 demarchi: yes Jan 28 19:27:23 denkenz: ping Jan 28 19:31:57 denkenz: for the NULL check removal patch, is it to send one patch for all the files inside plugin directory Jan 28 19:39:13 yes Jan 28 19:57:28 Jeevaka: Btw. the STE radio settings should be at insider drivers/mbmmodem/ since they are identical ;) Jan 28 19:57:53 Any you are right, that one is broken. No idea on how slipped through review. Jan 28 19:58:28 According to the commit log it is my fault. Jan 28 23:10:09 denkenz: Any problem with making another release? Jan 28 23:38:57 go for it **** ENDING LOGGING AT Sat Jan 29 02:59:56 2011