DDB operators patch

[pfg]

Summary

a patch I made almost 2 years ago which adds a bunch of new operators to DDB command language. I used back then to debug an issue I had with a network driver, but the intent of the code alays was to be the first pass towards supporting conditional breakpoint in DDB

[pfg]

I did some cleanups that were incorporated but I didn't test it.
We should push it for FreeBSD 11, I think.

[jhb]

sys/ddb/db_expr.c

213

This seems like a mismerge, and it also seems better served by using a switch with cases instead?

[dan_partelly_rdsor.ro]

@jhb true. Ill convert it to a case and fix the issue. I can also apply it on current and test it there. Once its fixed, what is the prefered course of action ?

[pfg]

In D4230#127621, @dan_partelly_rdsor.ro wrote:

@jhb true. Ill convert it to a case and fix the issue. I can also apply it on current and test it there. Once its fixed, what is the prefered course of action ?

Just update it. As soon as it gets some positive review it will get committed.

[dan_partelly_rdsor.ro]

1. Updating D4230: DDB operators patch #
2. Enter a brief description of the changes included in this update.
3. The first line is used as subject, next lines as comment. #
4. If you intended to create a new revision, use:
5. $ arc diff --create

Applied the patch the current, previously it was applied to a very old version
Changed db_add_expr function to use case statements
Fixed what I considered to be a bug in how db_print_cmd handles print formats.
Previously an incorrect format specifier whould not be cought and the invalid
specifier saved in db_print_format global varibale, altering in a unwanted way
the behaviour of subsequent calls to db_printf

kernel compiled cleanly after the patch was applied

[pfg]

I have a complementary enhancement from DragonFly: D6146

It would be really nice to document the new features (not sure where).

[pfg]

Now that the ddb examine (x) command changes are in, I would like to bring this change. It would be a step forward for FBSD 11.

There are some stray spaces which I will fix soon.

[jhb]

It would be good to document the expression syntax in ddb(4) if possible. For example, that would be the best place to mention that >> assumes unsigned.

sys/ddb/db_expr.c

57

It would be good to avoid mixing whitespace changes in. We can always fix those as a followup if needed, but this obscures the diff. Alternatively, you could reindent this function to match the rest of the file first and then update this diff so it just includes the functional changes.

92

On the one hand I appreciate the more verbose syntax error messages (and I probably come down on the side of keeping them). On the other hand, having a single string saves a small amount of space in the kernel .rodata section.

166

Inconsistent indentation here.

205

Ah, if you used this here, you could use this above in db_mult_expr() as well to pacify others who are worried about the space bloat of verbose error messages. :)

For example:

db_printf("Expression syntax error after '%c'\n", '!');

216

Perhaps use 'default' here to pacify compiler warnings about unused cases that aren't actually possible?

281

Indentation.

320

If we're going the space-savings route, then perhaps use strings instead of characters for the argument:

db_error("Expression syntax error after '%s'\n", "&&");

Note that this use would mean using the same quotes in all cases, but I think that the consistency of that would be worth it.

sys/ddb/db_lex.h

39

This is actually permitted in style(9), so I would leave this be.

[pfg]

Attempt to reduce diff.
Address some of jhb's concerns.

[pfg]

More updates:

Attempt to maintain existing Mach format.

[pfg]

I haven't figure out the strings yet.

sys/ddb/db_expr.c

216

I think this corresponded to line 220, where I added

	__unreachable();

now.

[jhb]

sys/ddb/db_expr.c

216

Yes. There's another one around 301 as well.

[dan_partelly_rdsor.ro]

Im still learning the FreeBSD style, Ill be more careful next time with indentation and spaces. Meanwhile, if I have some questions on
style , can I mail you Pedro ? About documenting, yes maybe next week I could work on man page, but it will have to wait untill then.

[jhb]

In D4230#131833, @dan_partelly_rdsor.ro wrote:

Im still learning the FreeBSD style, Ill be more careful next time with indentation and spaces. Meanwhile, if I have some questions on
style , can I mail you Pedro ? About documenting, yes maybe next week I could work on man page, but it will have to wait untill then.

Don't worry, this is how we learn it (unfortunately) and they are easily addressed.

[pfg]

In D4230#131833, @dan_partelly_rdsor.ro wrote:

Im still learning the FreeBSD style, Ill be more careful next time with indentation and spaces. Meanwhile, if I have some questions on
style , can I mail you Pedro ?

Well, this is not style(9), the code originated in Mach so we are respecting the previous style in order to minimize the differences. We probably should reformat this but the other BSDs have some interesting changes as well.

No problem about mailing me, except that I may take a while to reply.

[pfg]

• Add extra __unreachable()
• Make strings consistent

*untested*

[pfg]

Fix compilation.

Not sure if this is the correct approach but since db_error() doesn't
let me print messages with extra characters, I use db_printf()
followed by a db_error(NULL).

Not sure what db_print_format() does (?).

[pfg]

In D4230#131942, @pfg wrote:

...

Not sure what db_print_format() does (?).

Nevermind .. I opengroked it.

[pfg]

sync with current (the x/a option change was reverted).

[pfg]

Dan ..

Were these supposed to come from Mach? I can't find them in Mach3 or in Darwin. Just curious.

[dan_partelly_rdsor.ro]

@Pedro. Yes, Pedro , they where brought over from a Mach kernel. I believe it was 3.0 . Then applied some bug fixes on the recursive parser.

[dan_partelly_rdsor.ro]

Pedro, you asked for licensing reasons too ? Should I try to find exactly the version I used ?

[pfg]

In D4230#133771, @dan_partelly_rdsor.ro wrote:

Pedro, you asked for licensing reasons too ? Should I try to find exactly the version I used ?

No licensing issue. I was wondering about the convenience of the grammar. Once we add functions we cannot really take them back. Having the historical backing helps.

OTOH, I won't commit it until someone else approves it.

[adrian]

i'm okay with this. thanks for being so patient!

[pfg]

OK.. I've waited some prudential time but it's time to commit this.
Still pending are the documentation updates(?).

[dan_partelly_rdsor.ro]

@Pedro, this weekend I reserved time for doc updates

[pfg]

This was committed as r299970.