Patch against libcli 1 10 0 rel2 #49
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sync w/upsteam
… additional optargs
…LI_CMD_OPTION_MULTIPLE' flag set
All help lines consist of the name and helpstr. This treats allows each
line to be 'wrapped' and collimated so each partial piece of helpstr will
be left aligned with each other. Wrapping is done assuming an 80 col width,
and if a line is wrapped it will attempt to split it at the last white
space in that segment. If none found, then it prints what it can. Then it
also breaks at the first '\n' or '\r'.
Additionally, for optarg help, we can have 'specific' help messages in
addition to the general help for the optarg. Look at the clitest program
for an example (both 'triangle' and 'rectangle' have specific help msgs
for the 'shape' optarg. These specific help messages are tab ('\t')
separated and occur after the general help message. A specific message
will only be shown if 'could' be a match.
dparrish
requested changes
Aug 8, 2019
libcli.c
Outdated
| @@ -166,6 +166,7 @@ static int cli_int_execute_pipeline(struct cli_def *cli, struct cli_pipeline *pi | |||
| inline void cli_int_show_pipeline(struct cli_def *cli, struct cli_pipeline *pipeline); | |||
| static void cli_int_free_pipeline(struct cli_pipeline *pipeline); | |||
| static void cli_register_command_core(struct cli_def *cli, struct cli_command *parent, struct cli_command *c); | |||
| static void cli_int_wrap_help_line (char *nameptr, char *helpptr, struct cli_comphelp *comphelp) ; | |||
There was a problem hiding this comment.
Remove extra spaces around ( and )
libcli.c
Outdated
| * Attemp quick dirty wrapping of helptext taking into account the offset from name, embedded | ||
| * cr/lf in helptext, and trying to split on last white-text before the margin | ||
| */ | ||
| void cli_int_wrap_help_line (char *nameptr, char *helpptr, struct cli_comphelp *comphelp) { |
libcli.c
Outdated
| if (toprint > availwidth) { | ||
| toprint = availwidth; | ||
| while ((toprint>=0) && !isspace(helpptr[toprint])) toprint--; | ||
| if ( toprint < 0) { |
There was a problem hiding this comment.
Remove extra space after (.
BTW, even old versions of clang-format would do this for you. In fact, clang-format makes quite a lot of changes to this file.
There was a problem hiding this comment.
Yeah, one of these days when I (re)discover that mythical thing called 'free time' I need to spin up a machine that has a more recent version of clang than CentOS7. Right now our primary development platform is specific to RHEL7.5 - and our dev builds tend to be CentOS7 based. Our product is currently aimed as an appliance where we supply both H/W and S/W.
libcli.c
Outdated
|
|
||
| if (asprintf(&line, "%*.*s%.*s", namewidth, namewidth, nameptr, toprint, helpptr) < 0) break; | ||
| cli_add_comphelp_entry(comphelp, line); | ||
| if (nameptr != emptystring) { |
There was a problem hiding this comment.
Tiny little nitpick, but I don't think this if statement should be here. Just setting nameptr = emptystring every time is potentially more efficient.
- It's shorter code
nameptrwill equalemptystringafter the first loop so thisifstatement will always run after the first loop. If there are more than 2 lines in the output then it's more efficient not to have theif.
Yes it's a pedantic request. I think i thought about this too much just now and I feel silly.
There was a problem hiding this comment.
This from code creep. Originally this routine was tucked inside another routine where I'd've done a 'free_z(nameptr)' just before setting nameptr back to emptystring. When I moved this code to the new cli_int_wrap_help_line() I no longer needed the free (parent handled that), so I just popped the line. At any rate - fixed.
There was a problem hiding this comment.
Juggled order of code here also so the calls to asprintf, cli_add_comphelp_entry, and the free of said asprintf are consecutive.
RobSanders
commented
Aug 8, 2019
dparrish
approved these changes
Aug 8, 2019
|
Free time is what comes after your 120% job and the 50% on personal
projects!
…On Thu, Aug 8, 2019 at 9:49 PM Rob Sanders ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In libcli.c
<#49 (comment)>:
> + namewidth = strlen(nameptr);
+ availwidth = maxwidth - namewidth;
+
+ if (!helpptr) helpptr = emptystring;
+ /*
+ * Now we need to iterate one or more times to only print out at most
+ * maxwidth - leftwidth characters of helpptr. Note that there are no
+ * tabs in helpptr, so each 'char' displays as one char
+ */
+
+ do {
+ toprint = strlen(helpptr);
+ if (toprint > availwidth) {
+ toprint = availwidth;
+ while ((toprint>=0) && !isspace(helpptr[toprint])) toprint--;
+ if ( toprint < 0) {
Yeah, one of these days when I (re)discover that mythical thing called
'free time' I need to spin up a machine that has a more recent version of
clang than CentOS7. Right now our primary development platform is specific
to RHEL7.5 - and our dev builds tend to be CentOS7 based. Our product is
currently aimed as an appliance where we supply both H/W and S/W.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#49?email_source=notifications&email_token=AABPIQ7PB2BLOYG65C4PVXTQDQB4FA5CNFSM4IKDQ5V2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCA7EKHI#discussion_r311992628>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABPIQ2HMCVI3RVQ7OQRF73QDQB4FANCNFSM4IKDQ5VQ>
.
|
|
Concur - I went off and did so as well 😊
For reference…
https://en.cppreference.com/w/c/string/byte/isspace
Still owe you some documentation updates and some tweaks for limiting the number of ‘completions’ shown. And I’ve got an idea that I need to tweak the way I’m matching those ‘discrete’ help options….
--
ROBERT SANDERS
Sr. Secure Systems Engineer
FORCEPOINT
T +1.703.896.4762
F +1.703.318.5041
www.forcepoint.com
FORWARD WITHOUT FEAR
From: David Parrish <notifications@github.com>
Reply-To: dparrish/libcli <reply@reply.github.com>
Date: Thursday, August 8, 2019 at 9:55 AM
To: dparrish/libcli <libcli@noreply.github.com>
Cc: Rob Sanders <rsanders@forcepoint.com>, Author <author@noreply.github.com>
Subject: EXTERNAL: Re: [dparrish/libcli] Patch against libcli 1 10 0 rel2 (#49)
@dparrish commented on this pull request.
________________________________
In libcli.c<#49 (comment)>:
+ // crlf is a pointer - see if it is 'before' the toprint index
+ if ((crlf-helpptr) < toprint) {
+ // ok, crlf is before the wrap, us it
+ toprint = (crlf-helpptr);
+ }
+ }
+
+ if (asprintf(&line, "%*.*s%.*s", namewidth, namewidth, nameptr, toprint, helpptr) < 0) break;
+ cli_add_comphelp_entry(comphelp, line);
+ if (nameptr != emptystring) {
+ nameptr = emptystring;
+ }
+ free_z(line);
+ helpptr += toprint;
+ // advance to first non whitespace
+ while (helpptr && isspace(*helpptr)) helpptr++;
Sounds good. Still good to check!
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub<#49?email_source=notifications&email_token=AHHFTFZMSQG6FG6YFWJ5JMDQDQQVNA5CNFSM4IKDQ5V2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCA7V4WI#discussion_r312048529>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AHHFTF6TKP63NKSICA37I3DQDQQVNANCNFSM4IKDQ5VQ>.
|
|
David,
Are you good with a full merge to stable from the branch I submitted? I did the initial merge request to a new branch just to keep it clean from stable until we were ready. I started to tag things as V1.10.1, but had to back that when I noticed that the merge was accepted to that staging branch.
-R
--
ROBERT SANDERS
Sr. Secure Systems Engineer
FORCEPOINT
T +1.703.896.4762
F +1.703.318.5041
www.forcepoint.com
FORWARD WITHOUT FEAR
From: David Parrish <notifications@github.com>
Reply-To: dparrish/libcli <reply@reply.github.com>
Date: Thursday, August 8, 2019 at 9:58 AM
To: dparrish/libcli <libcli@noreply.github.com>
Cc: Rob Sanders <rsanders@forcepoint.com>, Mention <mention@noreply.github.com>
Subject: EXTERNAL: Re: [dparrish/libcli] Patch against libcli 1 10 0 rel2 (#49)
Free time is what comes after your 120% job and the 50% on personal
projects!
On Thu, Aug 8, 2019 at 9:49 PM Rob Sanders ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In libcli.c
<#49 (comment)>:
> + namewidth = strlen(nameptr);
+ availwidth = maxwidth - namewidth;
+
+ if (!helpptr) helpptr = emptystring;
+ /*
+ * Now we need to iterate one or more times to only print out at most
+ * maxwidth - leftwidth characters of helpptr. Note that there are no
+ * tabs in helpptr, so each 'char' displays as one char
+ */
+
+ do {
+ toprint = strlen(helpptr);
+ if (toprint > availwidth) {
+ toprint = availwidth;
+ while ((toprint>=0) && !isspace(helpptr[toprint])) toprint--;
+ if ( toprint < 0) {
Yeah, one of these days when I (re)discover that mythical thing called
'free time' I need to spin up a machine that has a more recent version of
clang than CentOS7. Right now our primary development platform is specific
to RHEL7.5 - and our dev builds tend to be CentOS7 based. Our product is
currently aimed as an appliance where we supply both H/W and S/W.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#49?email_source=notifications&email_token=AABPIQ7PB2BLOYG65C4PVXTQDQB4FA5CNFSM4IKDQ5V2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCA7EKHI#discussion_r311992628>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABPIQ2HMCVI3RVQ7OQRF73QDQB4FANCNFSM4IKDQ5VQ>
.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#49?email_source=notifications&email_token=AHHFTF675BUGAAAKGPG27YTQDQQ7HA5CNFSM4IKDQ5V2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD33WJOA#issuecomment-519529656>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AHHFTF4WMGTPXU4W6SDP523QDQQ7HANCNFSM4IKDQ5VQ>.
|
|
Actually, hold off on merge to stable…. speaking with one of my teammates on ‘usability’ of the optarg help stuff (currently explicitly tab separated) and are musing changing the API (and possibly inner workings) to not use the \t as the field specifier.
opt 1- maintain ‘\t’, but add new ‘cli_optarg_add_help(optarg_ptr, namestr, textstr)’ call that concats everything together using tabs as separaters
opt 2- new struct cli_help_entry { char *name, char *text, struct cli_help_entry *next) and use this rather than the ‘on-the-fly’ processing like now.
-R
--
ROBERT SANDERS
Sr. Secure Systems Engineer
FORCEPOINT
T +1.703.896.4762
F +1.703.318.5041
www.forcepoint.com
FORWARD WITHOUT FEAR
From: "Sanders, Robert" <rsanders@forcepointgov.com>
Date: Friday, August 9, 2019 at 7:37 AM
To: dparrish/libcli <reply@reply.github.com>, dparrish/libcli <libcli@noreply.github.com>
Cc: Rob Sanders <rsanders@forcepoint.com>, Mention <mention@noreply.github.com>
Subject: Re: EXTERNAL: Re: [dparrish/libcli] Patch against libcli 1 10 0 rel2 (#49)
David,
Are you good with a full merge to stable from the branch I submitted? I did the initial merge request to a new branch just to keep it clean from stable until we were ready. I started to tag things as V1.10.1, but had to back that when I noticed that the merge was accepted to that staging branch.
-R
--
ROBERT SANDERS
Sr. Secure Systems Engineer
FORCEPOINT
T +1.703.896.4762
F +1.703.318.5041
www.forcepoint.com
FORWARD WITHOUT FEAR
From: David Parrish <notifications@github.com>
Reply-To: dparrish/libcli <reply@reply.github.com>
Date: Thursday, August 8, 2019 at 9:58 AM
To: dparrish/libcli <libcli@noreply.github.com>
Cc: Rob Sanders <rsanders@forcepoint.com>, Mention <mention@noreply.github.com>
Subject: EXTERNAL: Re: [dparrish/libcli] Patch against libcli 1 10 0 rel2 (#49)
Free time is what comes after your 120% job and the 50% on personal
projects!
On Thu, Aug 8, 2019 at 9:49 PM Rob Sanders ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In libcli.c
<#49 (comment)>:
> + namewidth = strlen(nameptr);
+ availwidth = maxwidth - namewidth;
+
+ if (!helpptr) helpptr = emptystring;
+ /*
+ * Now we need to iterate one or more times to only print out at most
+ * maxwidth - leftwidth characters of helpptr. Note that there are no
+ * tabs in helpptr, so each 'char' displays as one char
+ */
+
+ do {
+ toprint = strlen(helpptr);
+ if (toprint > availwidth) {
+ toprint = availwidth;
+ while ((toprint>=0) && !isspace(helpptr[toprint])) toprint--;
+ if ( toprint < 0) {
Yeah, one of these days when I (re)discover that mythical thing called
'free time' I need to spin up a machine that has a more recent version of
clang than CentOS7. Right now our primary development platform is specific
to RHEL7.5 - and our dev builds tend to be CentOS7 based. Our product is
currently aimed as an appliance where we supply both H/W and S/W.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#49?email_source=notifications&email_token=AABPIQ7PB2BLOYG65C4PVXTQDQB4FA5CNFSM4IKDQ5V2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCA7EKHI#discussion_r311992628>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABPIQ2HMCVI3RVQ7OQRF73QDQB4FANCNFSM4IKDQ5VQ>
.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#49?email_source=notifications&email_token=AHHFTF675BUGAAAKGPG27YTQDQQ7HA5CNFSM4IKDQ5V2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD33WJOA#issuecomment-519529656>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AHHFTF4WMGTPXU4W6SDP523QDQQ7HANCNFSM4IKDQ5VQ>.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Several bug fixes, but major work on how 'help' messages are display. Biggest change for 'legacy' is that the help messages for commands are now wrapped (default 80 cols), and my contain embedded cr/lf