DavidSpickett wrote: > I agree with all the problems you listed above, but, while for now this is > just for testing purposes, it is a legitimately useful method for any clients > of the SB API.
I agree it has the look of something useful but if you didn't need it for testing I don't think that alone would justify including it. > One thing we could do is set the SBError object to an error state if any > string read failed. This way clients would know they can't trust an empty > string result. What do you think? Depends on how they could then react to it. Looking at https://lldb.llvm.org/python_api/lldb.SBProcess.html#lldb.SBProcess.ReadCStringFromMemory, they could call this on any string that came back empty. Though they might not know that partial reads aren't really a thing, so they may call it on all of them, which is fine but less efficient. * ReadCStrings from A B and C * If error is failure * ReadCString A * Check error * Repeat for B and C Maybe we could use SBStructuredData? Though I don't see a way for a client to discover the length of a contained string value. `GetStringValue` takes a length but you'd have to keep guessing larger and larger numbers? I have not used this type before though so I could be missing something. Also I don't see a way to put errors in it. We could say oh if you find this particular type then it's an error string but that's getting very fiddly. You could look at other structured data APIs for inspiration. > We need to accept a 0-length string, I'm not suggesting otherwise, but I am > displeased by the state we've gotten ourselves in. There's an argument that says that things that look like `ReadCString` (singular) should be exactly as rubbish as `ReadCString` so they they are familiar in both name and behaviour. Otherwise our attempts to make slight improvements just make it more confusing as each variant needs different handling. The drawback is you assign the most perfect, obvious name, to the one with the awkward handling. Then we inevitably add `ReadCStringEx` which is actually just `ReadCStringsEx(<list of one>)`, but at some point this is just how APIs go isn't it. My point is that if you make too much effort it actually gets harder to use. So even though "failure error means don't trust zero length string" is a bit of a hack, maybe it's just enough of a hack :) . https://github.com/llvm/llvm-project/pull/172026 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
