* Re: minor trinity patch
[not found] ` <20121220201903.GA14944@redhat.com>
@ 2013-05-22 14:14 ` Nico Golde
2013-05-23 18:58 ` Tommi Rantala
0 siblings, 1 reply; 6+ messages in thread
From: Nico Golde @ 2013-05-22 14:14 UTC (permalink / raw)
To: trinity
[-- Attachment #1: Type: text/plain, Size: 2684 bytes --]
Hi,
I'm not sure if you would like to change this....
In devices.c you scan for the device name in /proc/devices using the %as
string.
On bionic this doesn't seem to work and therefore no devices are found and
because of this no ioctl group ever matches.
I changed the code the following way:
@@ -21,7 +22,8 @@ static size_t bldevs, chrdevs, miscdevs;
static void parse_proc_devices(void)
{
FILE *fp;
- char *name, *line = NULL;
+ char *line = NULL;
+ char name[32];
size_t n = 0;
int block, major;
void *new;
@@ -33,30 +35,31 @@ static void parse_proc_devices(void)
block = 0;
while (getline(&line, &n, fp) >= 0) {
+ memset(name, 0, sizeof(name));
if (strcmp("Block devices:\n", line) == 0)
block = 1;
- else if (sscanf(line, "%d %as", &major, &name) == 2) {
+ else if (strcmp("Character devices:\n", line) == 0)
+ block = 0;
+ else if (sscanf(line, "%d %s", &major, &name) == 2) {
if (block) {
new = realloc(block_devs, (bldevs+1)*sizeof(*block_devs));
if (!new) {
- free(name);
continue;
}
block_devs = new;
block_devs[bldevs].major = major;
block_devs[bldevs].minor = 0;
- block_devs[bldevs].name = name;
+ block_devs[bldevs].name = strdup(name);
bldevs++;
} else {
new = realloc(char_devs, (chrdevs+1)*sizeof(*char_devs));
if (!new) {
- free(name);
continue;
}
char_devs = new;
char_devs[chrdevs].major = major;
char_devs[chrdevs].minor = 0;
- char_devs[chrdevs].name = name;
+ char_devs[chrdevs].name = strdup(name);
chrdevs++;
}
}
I also added the additional strcmp just to make sure that the code works also
if the order of character devices/block devices in /proc/devices changes at
some point.
Cheers
Nico
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: minor trinity patch
2013-05-22 14:14 ` minor trinity patch Nico Golde
@ 2013-05-23 18:58 ` Tommi Rantala
2013-05-23 23:47 ` Nico Golde
0 siblings, 1 reply; 6+ messages in thread
From: Tommi Rantala @ 2013-05-23 18:58 UTC (permalink / raw)
To: Nico Golde; +Cc: trinity
2013/5/22 Nico Golde <nico@ngolde.de>:
> Hi,
> I'm not sure if you would like to change this....
> In devices.c you scan for the device name in /proc/devices using the %as
> string.
> On bionic this doesn't seem to work and therefore no devices are found and
> because of this no ioctl group ever matches.
No objections to changing this.
> I changed the code the following way:
> @@ -21,7 +22,8 @@ static size_t bldevs, chrdevs, miscdevs;
> static void parse_proc_devices(void)
> {
> FILE *fp;
> - char *name, *line = NULL;
> + char *line = NULL;
> + char name[32];
It would be IMO preferable to not hardcode the max length.
> size_t n = 0;
> int block, major;
> void *new;
> @@ -33,30 +35,31 @@ static void parse_proc_devices(void)
> block = 0;
>
> while (getline(&line, &n, fp) >= 0) {
> + memset(name, 0, sizeof(name));
> if (strcmp("Block devices:\n", line) == 0)
> block = 1;
> - else if (sscanf(line, "%d %as", &major, &name) == 2) {
> + else if (strcmp("Character devices:\n", line) == 0)
> + block = 0;
> + else if (sscanf(line, "%d %s", &major, &name) == 2) {
> if (block) {
> new = realloc(block_devs, (bldevs+1)*sizeof(*block_devs));
> if (!new) {
> - free(name);
> continue;
> }
> block_devs = new;
> block_devs[bldevs].major = major;
> block_devs[bldevs].minor = 0;
> - block_devs[bldevs].name = name;
> + block_devs[bldevs].name = strdup(name);
> bldevs++;
> } else {
> new = realloc(char_devs, (chrdevs+1)*sizeof(*char_devs));
> if (!new) {
> - free(name);
> continue;
> }
> char_devs = new;
> char_devs[chrdevs].major = major;
> char_devs[chrdevs].minor = 0;
> - char_devs[chrdevs].name = name;
> + char_devs[chrdevs].name = strdup(name);
> chrdevs++;
> }
> }
>
>
> I also added the additional strcmp just to make sure that the code works also
> if the order of character devices/block devices in /proc/devices changes at
> some point.
>
> Cheers
> Nico
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: minor trinity patch
2013-05-23 18:58 ` Tommi Rantala
@ 2013-05-23 23:47 ` Nico Golde
2013-06-03 5:55 ` Tommi Rantala
0 siblings, 1 reply; 6+ messages in thread
From: Nico Golde @ 2013-05-23 23:47 UTC (permalink / raw)
To: trinity
Hi,
* Tommi Rantala <tt.rantala@gmail.com> [2013-05-23 20:58]:
> 2013/5/22 Nico Golde <nico@ngolde.de>:
> > I changed the code the following way:
> > @@ -21,7 +22,8 @@ static size_t bldevs, chrdevs, miscdevs;
> > static void parse_proc_devices(void)
> > {
> > FILE *fp;
> > - char *name, *line = NULL;
> > + char *line = NULL;
> > + char name[32];
>
> It would be IMO preferable to not hardcode the max length.
I agree, this is actually left over from quickly changing this.
Are you aware of a proper maximum defined in the kernel headers or do we want to
allocate this completely dynamically?
Cheers
Nico
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: minor trinity patch
2013-05-23 23:47 ` Nico Golde
@ 2013-06-03 5:55 ` Tommi Rantala
2013-06-21 13:20 ` Nico Golde
0 siblings, 1 reply; 6+ messages in thread
From: Tommi Rantala @ 2013-06-03 5:55 UTC (permalink / raw)
To: Nico Golde; +Cc: trinity
2013/5/24 Nico Golde <nico@ngolde.de>:
> Hi,
> * Tommi Rantala <tt.rantala@gmail.com> [2013-05-23 20:58]:
>> 2013/5/22 Nico Golde <nico@ngolde.de>:
>> > I changed the code the following way:
>> > @@ -21,7 +22,8 @@ static size_t bldevs, chrdevs, miscdevs;
>> > static void parse_proc_devices(void)
>> > {
>> > FILE *fp;
>> > - char *name, *line = NULL;
>> > + char *line = NULL;
>> > + char name[32];
>>
>> It would be IMO preferable to not hardcode the max length.
>
> I agree, this is actually left over from quickly changing this.
> Are you aware of a proper maximum defined in the kernel headers or do we want to
> allocate this completely dynamically?
dynamic please!
> Cheers
> Nico
> --
> To unsubscribe from this list: send the line "unsubscribe trinity" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: minor trinity patch
2013-06-03 5:55 ` Tommi Rantala
@ 2013-06-21 13:20 ` Nico Golde
2013-06-24 16:03 ` Nico Golde
0 siblings, 1 reply; 6+ messages in thread
From: Nico Golde @ 2013-06-21 13:20 UTC (permalink / raw)
To: trinity
[-- Attachment #1.1: Type: text/plain, Size: 966 bytes --]
Hi,
* Tommi Rantala <tt.rantala@gmail.com> [2013-06-03 07:56]:
> 2013/5/24 Nico Golde <nico@ngolde.de>:
> > * Tommi Rantala <tt.rantala@gmail.com> [2013-05-23 20:58]:
> >> 2013/5/22 Nico Golde <nico@ngolde.de>:
> >> > I changed the code the following way:
> >> > @@ -21,7 +22,8 @@ static size_t bldevs, chrdevs, miscdevs;
> >> > static void parse_proc_devices(void)
> >> > {
> >> > FILE *fp;
> >> > - char *name, *line = NULL;
> >> > + char *line = NULL;
> >> > + char name[32];
> >>
> >> It would be IMO preferable to not hardcode the max length.
> >
> > I agree, this is actually left over from quickly changing this.
> > Are you aware of a proper maximum defined in the kernel headers or do we want to
> > allocate this completely dynamically?
>
> dynamic please!
Sorry I had no time to look into this for a while.
Patch attached.
Cheers
Nico
--
Nico Golde - XMPP: nion@jabber.ccc.de - GPG: 0xA0A0AAAA
[-- Attachment #1.2: 0001-devices.c-dont-use-as-scanf-format-string-but-instea.patch --]
[-- Type: text/x-diff, Size: 1287 bytes --]
From 9acc7c59c6ae2643ffba4edc17b04477f5cdaee0 Mon Sep 17 00:00:00 2001
From: Nico Golde <nion@debian.org>
Date: Fri, 21 Jun 2013 15:18:58 +0200
Subject: [PATCH] devices.c: dont use %as scanf format string, but instead
allocate and parse the device name manually for
compatibility with non-glibc libcs
---
devices.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/devices.c b/devices.c
index 43433bc..d3916f9 100644
--- a/devices.c
+++ b/devices.c
@@ -21,7 +21,7 @@ static size_t bldevs, chrdevs, miscdevs;
static void parse_proc_devices(void)
{
FILE *fp;
- char *name, *line = NULL;
+ char *p, *name, *line = NULL;
size_t n = 0;
int block, major;
void *new;
@@ -35,7 +35,14 @@ static void parse_proc_devices(void)
while (getline(&line, &n, fp) >= 0) {
if (strcmp("Block devices:\n", line) == 0)
block = 1;
- else if (sscanf(line, "%d %as", &major, &name) == 2) {
+ else if (strcmp("Character devices:\n", line) == 0)
+ block = 0;
+ else if (sscanf(line, "%d %*s", &major) == 1) {
+ if ((p = strrchr(line, ' ')) == NULL)
+ continue;
+ p++;
+ name = strdup(p);
+
if (block) {
new = realloc(block_devs, (bldevs+1)*sizeof(*block_devs));
if (!new) {
--
1.7.10.4
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: minor trinity patch
2013-06-21 13:20 ` Nico Golde
@ 2013-06-24 16:03 ` Nico Golde
0 siblings, 0 replies; 6+ messages in thread
From: Nico Golde @ 2013-06-24 16:03 UTC (permalink / raw)
To: trinity
[-- Attachment #1.1: Type: text/plain, Size: 458 bytes --]
Hi,
* Nico Golde <nico@ngolde.de> [2013-06-21 15:20]:
> Sorry I had no time to look into this for a while.
> Patch attached.
Attached is another patch on top of the previous one. I didn't see that the
device name includes the newline so that map_dev() will fail.
This was kind of hard to see unless you have a driver and you know how the
command ids should look like.
Thanks
Nico
--
Nico Golde - XMPP: nion@jabber.ccc.de - GPG: 0xA0A0AAAA
[-- Attachment #1.2: 0001-parse_proc_devices-cut-off-device-string-on-newline.patch --]
[-- Type: text/x-diff, Size: 707 bytes --]
From f850d64ba34c0a0d203912740c00fff03e8f8f3e Mon Sep 17 00:00:00 2001
From: Nico Golde <nion@debian.org>
Date: Mon, 24 Jun 2013 17:56:19 +0200
Subject: [PATCH] parse_proc_devices: cut off device string on newline
---
devices.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/devices.c b/devices.c
index d3916f9..0166a53 100644
--- a/devices.c
+++ b/devices.c
@@ -38,6 +38,8 @@ static void parse_proc_devices(void)
else if (strcmp("Character devices:\n", line) == 0)
block = 0;
else if (sscanf(line, "%d %*s", &major) == 1) {
+ if ((p = strchr(line, '\n')) != NULL)
+ *p = 0;
if ((p = strrchr(line, ' ')) == NULL)
continue;
p++;
--
1.7.10.4
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-06-24 16:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20121220165100.GA8748@ngolde.de>
[not found] ` <20121220171726.GA7543@redhat.com>
[not found] ` <20121220172246.GA16285@ngolde.de>
[not found] ` <20121220172741.GA17760@redhat.com>
[not found] ` <20121220200255.GB16285@ngolde.de>
[not found] ` <20121220201903.GA14944@redhat.com>
2013-05-22 14:14 ` minor trinity patch Nico Golde
2013-05-23 18:58 ` Tommi Rantala
2013-05-23 23:47 ` Nico Golde
2013-06-03 5:55 ` Tommi Rantala
2013-06-21 13:20 ` Nico Golde
2013-06-24 16:03 ` Nico Golde
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox