We should specify what kind of data are send to snprintf even at
least as (char *).
---
OK, I am not certain with this one. Is it (char *) or (CommandDef *)?
Matěj
vis-cmds.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/vis-cmds.c b/vis-cmds.c
index 80040921..6046a874 100644
--- a/vis-cmds.c+++ b/vis-cmds.c
@@ -704,7 +704,7 @@ static bool print_cmd_name(const char *key, void *value, void *data) {
size_t len_data = strlen(data);
size_t remaining_capacity = VIS_COMMAND_BUFFER_MAX - len_data;
if (remaining_capacity >= strlen(cmd->name) + 2) /* adding 2 for \n and \0 */
- snprintf(data + len_data, remaining_capacity, "%s\n", cmd->name);+ snprintf((char *)data + len_data, remaining_capacity, "%s\n", cmd->name); return true;
}
--
2.44.0
Matěj Cepl <mcepl@cepl.eu> wrote:
> We should specify what kind of data are send to snprintf even at> least as (char *).> ---> OK, I am not certain with this one. Is it (char *) or (CommandDef *)?
The man page for "snprintf" says that it takes a (char *), so I think
the patch is correct.
Cheers,
Silvan
> > Matěj> > vis-cmds.c | 2 +-> 1 file changed, 1 insertion(+), 1 deletion(-)> > diff --git a/vis-cmds.c b/vis-cmds.c> index 80040921..6046a874 100644> --- a/vis-cmds.c> +++ b/vis-cmds.c> @@ -704,7 +704,7 @@ static bool print_cmd_name(const char *key, void *value, void *data) {> size_t len_data = strlen(data);> size_t remaining_capacity = VIS_COMMAND_BUFFER_MAX - len_data;> if (remaining_capacity >= strlen(cmd->name) + 2) /* adding 2 for \n and \0 */> - snprintf(data + len_data, remaining_capacity, "%s\n", cmd->name);> + snprintf((char *)data + len_data, remaining_capacity, "%s\n", cmd->name);> return true;> }>
Matěj Cepl <mcepl@cepl.eu> wrote:
> We should specify what kind of data are send to snprintf even at> least as (char *).> ---> OK, I am not certain with this one. Is it (char *) or (CommandDef *)?> > Matěj> > vis-cmds.c | 2 +-> 1 file changed, 1 insertion(+), 1 deletion(-)> > diff --git a/vis-cmds.c b/vis-cmds.c> index 80040921..6046a874 100644> --- a/vis-cmds.c> +++ b/vis-cmds.c> @@ -704,7 +704,7 @@ static bool print_cmd_name(const char *key, void *value, void *data) {> size_t len_data = strlen(data);> size_t remaining_capacity = VIS_COMMAND_BUFFER_MAX - len_data;> if (remaining_capacity >= strlen(cmd->name) + 2) /* adding 2 for \n and \0 */> - snprintf(data + len_data, remaining_capacity, "%s\n", cmd->name);> + snprintf((char *)data + len_data, remaining_capacity, "%s\n", cmd->name);> return true;> }>
Hi Matěj,
This doesn't seem to be part of vis? Perhaps its against some other
patch? Definitely (void *) arithmetic is nonsense but probably the
function signature should just be changed.
- Randy
--
https://rnpnr.xyz/
GPG Fingerprint: B8F0 CF4C B6E9 415C 1B27 A8C4 C8D2 F782 86DF 2DC5
On Tue Mar 26, 2024 at 1:35 AM CET, Randy Palamar wrote:
> > @@ -704,7 +704,7 @@ static bool print_cmd_name(const char *key, void *value, void *data) {> > size_t len_data = strlen(data);> > size_t remaining_capacity = VIS_COMMAND_BUFFER_MAX - len_data;> > if (remaining_capacity >= strlen(cmd->name) + 2) /* adding 2 for \n and \0 */> > - snprintf(data + len_data, remaining_capacity, "%s\n", cmd->name);> > + snprintf((char *)data + len_data, remaining_capacity, "%s\n", cmd->name);> > return true;>> This doesn't seem to be part of vis? Perhaps its against some other> patch? Definitely (void *) arithmetic is nonsense but probably the> function signature should just be changed.
The signature of print_cmd_name should not be changed, because
that would break whole map_iterate() stack. We need to do
recasting somewhere inside of the function, and that snprintf()
call seems like the proper place, unless we want to do something
more involved with the data value.
Best,
Matěj
--
http://matej.ceplovi.cz/blog/, @mcepl@floss.social
GPG Finger: 3C76 A027 CA45 AD70 98B5 BC1D 7920 5802 880B C9D8
A GOOD name is rather to be chosen than great riches.
-- Proverbs 22:1
On Tue Mar 26, 2024 at 1:35 AM CET, Randy Palamar wrote:
> This doesn't seem to be part of vis? Perhaps its against some other> patch?
And yes, you are right, it is from
https://github.com/martanne/vis/pull/1173 by Max Schillinger
(added to this message as CC, see the list archive for the rest
of the thread).
Best,
Matěj
--
http://matej.ceplovi.cz/blog/, @mcepl@floss.social
GPG Finger: 3C76 A027 CA45 AD70 98B5 BC1D 7920 5802 880B C9D8
Fascism is the stage reached after communism has proved an
illusion.
-- Friedrich von Hayek
Matěj Cepl <mcepl@cepl.eu> wrote:
> On Tue Mar 26, 2024 at 1:35 AM CET, Randy Palamar wrote:> > > @@ -704,7 +704,7 @@ static bool print_cmd_name(const char *key, void *value, void *data) {> > > size_t len_data = strlen(data);> > > size_t remaining_capacity = VIS_COMMAND_BUFFER_MAX - len_data;> > > if (remaining_capacity >= strlen(cmd->name) + 2) /* adding 2 for \n and \0 */> > > - snprintf(data + len_data, remaining_capacity, "%s\n", cmd->name);> > > + snprintf((char *)data + len_data, remaining_capacity, "%s\n", cmd->name);> > > return true;> >> > This doesn't seem to be part of vis? Perhaps its against some other> > patch? Definitely (void *) arithmetic is nonsense but probably the> > function signature should just be changed.> > The signature of print_cmd_name should not be changed, because> that would break whole map_iterate() stack. We need to do> recasting somewhere inside of the function, and that snprintf()> call seems like the proper place, unless we want to do something> more involved with the data value.
Yes I see now. While reading K&R[0] again recently I realized the
better thing to do in these situations is to cast the function
pointer so that you don't need to cast the parameters themselves
within the function. You can also use typedef to avoid typing it
every time:
```
typedef bool (*MapFunc)(const char *, void *, void *);
```
So later on it would look like:
```
static bool print_cmd_name(const char *key, CommendDef *cmd, char *cmdbuf);
map_iterate(vis->cmds, (MapFunc)print_cmd_name, cmd_list);
```
It seems many C hackers don't know about this trick. Personally I
think it improves readability.
- Randy
[0] B. W. Kerninghan and D. M. Ritchie, The C Programming Language
--
https://rnpnr.xyz/
GPG Fingerprint: B8F0 CF4C B6E9 415C 1B27 A8C4 C8D2 F782 86DF 2DC5
On Thu Mar 28, 2024 at 12:05 PM CET, Matěj Cepl wrote:
> On Tue Mar 26, 2024 at 1:35 AM CET, Randy Palamar wrote:> > This doesn't seem to be part of vis? Perhaps its against some other> > patch?>> And yes, you are right, it is from> https://github.com/martanne/vis/pull/1173 by Max Schillinger> (added to this message as CC, see the list archive for the rest> of the thread).>> Best,>> Matěj
Hi all,
thanks for pinging me. I saw this thread but I wasn't aware that it's
about my pull request.
Sorry for the void pointer arithmetic! Casting data to (char *) seems to
be the obvious fix to me. But Randy's proposal (casting the function)
sounds also good.
Best regards,
Max
On Sun Mar 31, 2024 at 9:43 PM CEST, Max Schillinger wrote:
> thanks for pinging me. I saw this thread but I wasn't aware that it's > about my pull request.
I wasn’t aware myself … Randy had to remind me that I was not
patching the master branch code.
> Sorry for the void pointer arithmetic! Casting data to (char *) seems to > be the obvious fix to me. But Randy's proposal (casting the function) > sounds also good.
Could we get an update of the pull request, please?
Thank you,
Matěj
--
http://matej.ceplovi.cz/blog/, @mcepl@floss.social
GPG Finger: 3C76 A027 CA45 AD70 98B5 BC1D 7920 5802 880B C9D8
Roses are red;
Violets are blue.
I’m schizophrenic,
And so am I.
On Tue Mar 26, 2024 at 1:35 AM CET, Randy Palamar wrote:
> This doesn't seem to be part of vis? Perhaps its against some other> patch? Definitely (void *) arithmetic is nonsense but probably the> function signature should just be changed.
This patch should be probably REJECTED, it has been incorporated
into https://github.com/martanne/vis/pull/1173.
Best,
Matěj
--
http://matej.ceplovi.cz/blog/, @mcepl@floss.social
GPG Finger: 3C76 A027 CA45 AD70 98B5 BC1D 7920 5802 880B C9D8
[…] a superior pilot uses his superior judgment to avoid having
to exercise his superior skill.
-- http://www.jwz.org/blog/2009/09/that-duct-tape-silliness/#comment-10653