GNOME Bugzilla – Bug 701329
[MM 0.8] [PATCH] +COPS=? response can contain quoted commas
Last modified: 2013-05-31 18:13:46 UTC
Created attachment 245700 [details] [review] 0001-modem-helpers-handle-commas-within-COPS-response-ite.patch +COPS: (1,"T-Mobile USA, In","T-Mobile","310260",0),(1,"AT&T","AT&T","310410",0),,(0,1,2,3,4),(0,1,2) Note the comma in the T-Mobile USA long string. Our regex for +COPS response uses the comma as a separator, so T-Mobile doesn't match and we only get one result. Patch attached, it's for 0.6 but I think it'll translate fairly well to master too. The big question: can we assume that modems always give the operator long name? I haven't seen a modem that doesn't, but I'm not 100% sure.
(In reply to comment #0) > Created an attachment (id=245700) [details] [review] > 0001-modem-helpers-handle-commas-within-COPS-response-ite.patch > > +COPS: (1,"T-Mobile USA, > In","T-Mobile","310260",0),(1,"AT&T","AT&T","310410",0),,(0,1,2,3,4),(0,1,2) > > Note the comma in the T-Mobile USA long string. Our regex for +COPS response > uses the comma as a separator, so T-Mobile doesn't match and we only get one > result. > > Patch attached, it's for 0.6 but I think it'll translate fairly well to master > too. The big question: can we assume that modems always give the operator long > name? I haven't seen a modem that doesn't, but I'm not 100% sure. Same here, not 100% sure, but I think we can assume that. If for some reason the modem doesn't give it, it will probably set the field to an empty string and keep the format anyway.
Review of attachment 245700 [details] [review]: ::: src/mm-modem-helpers.c @@ +182,3 @@ */ + r = g_regex_new ("\\((\\d),\"([^\"\\)]*)\",([^,\\)]*),([^\\)]*)\\)", G_REGEX_UNGREEDY, 0, &err); For the second regex, I don't think we can assume that the operator long name is always given as a quoted string. I see that this is what you probably meant in your comment before. If there are modems giving empty fields even for strings (i.e. not even ""), then we should probably also try to handle that...
Would you like me to skip the changes to the second regex? If we really want to handle this, we'll need more complex parsing of the response, probably by walking the string and handling the various chunks. I'm not really sure it's worth that, given that I've not seen an example of a device that returns an empty, non-quoted operator long string. If we do run into that device, then we could investigate changes to the parsing code...
(In reply to comment #3) > Would you like me to skip the changes to the second regex? > > If we really want to handle this, we'll need more complex parsing of the > response, probably by walking the string and handling the various chunks. I'm > not really sure it's worth that, given that I've not seen an example of a > device that returns an empty, non-quoted operator long string. If we do run > into that device, then we could investigate changes to the parsing code... Yeah, let's do that, if that device ever comes up we'll fix it. Fine for me the patch then.
Pushed to 0.6.
Ported to git master and pushed.