setStage2LocFromCmdline() doesn't need to do so much allocation just to
use strtok(), which should be cleansed with fire anyway. Get rid of
both.
---
loader/method.c | 68 +++++++++++++++++++++++++++---------------------------
1 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/loader/method.c b/loader/method.c
index 5f3a7ce..a49d01e 100644
--- a/loader/method.c
+++ b/loader/method.c
@@ -500,39 +500,39 @@ int getFileFromBlockDevice(char *device, char *path, char * dest) {
}

void setStage2LocFromCmdline(char * arg, struct loaderData_s * ld) {
- char * c, * dup;
-
- dup = strdup(arg);
- c = dup;
- /* : will let us delimit real information on the method */
- if ((c = strtok(c, ":"))) {
- c = strtok(NULL, ":");
-
- if (!strncmp(arg, "nfs:", 4)) {
- ld->method = METHOD_NFS;
- ld->stage2Data = calloc(sizeof(struct nfsInstallData *), 1);
-
- parseNfsHostPathOpts(arg + 4,
- &(((struct nfsInstallData *)ld->stage2Data)->host),
- &(((struct nfsInstallData *)ld->stage2Data)->directory),
- &(((struct nfsInstallData *)ld->stage2Data)->mountOpts));
- } else if (!strncmp(arg, "ftp:", 4) ||
- !strncmp(arg, "http", 4)) {
- ld->method = METHOD_URL;
- ld->stage2Data = calloc(sizeof(struct urlInstallData *), 1);
- ((urlInstallData *)ld->stage2Data)->url = strdup(arg);
- } else if (!strncmp(arg, "cdrom:", 6)) {
- ld->method = METHOD_CDROM;
- } else if (!strncmp(arg, "harddrive:", 10) ||
- !strncmp(arg, "hd:", 3)) {
- ld->method = METHOD_HD;
- ld->stage2Data = calloc(sizeof(struct hdInstallData *), 1);
- ((struct hdInstallData *)ld->stage2Data)->partition = strdup(c);
- if ((c = strtok(NULL, ":")))
- ((struct hdInstallData *)ld->stage2Data)->directory = strdup(c);
- else
- ((struct hdInstallData *)ld->stage2Data)->directory = NULL;
- }
+ if (!strncmp(arg, "nfs:", 4)) {
+ ld->method = METHOD_NFS;
+ ld->stage2Data = calloc(sizeof(struct nfsInstallData *), 1);
+
+ parseNfsHostPathOpts(arg + 4,
+ &(((struct nfsInstallData *)ld->stage2Data)->host),
+ &(((struct nfsInstallData *)ld->stage2Data)->directory),
+ &(((struct nfsInstallData *)ld->stage2Data)->mountOpts));
+ } else if (!strncmp(arg, "ftp:", 4) ||
+ !strncmp(arg, "http", 4)) {
+ ld->method = METHOD_URL;
+ ld->stage2Data = calloc(sizeof(struct urlInstallData *), 1);
+ ((urlInstallData *)ld->stage2Data)->url = strdup(arg);
+ } else if (!strncmp(arg, "cdrom:", 6)) {
+ ld->method = METHOD_CDROM;
+ } else if (!strncmp(arg, "harddrive:", 10) ||
+ !strncmp(arg, "hd:", 3)) {
+ size_t offset;
+
+ offset = strcspn(arg, ":");
+ arg += strcspn(arg, ":");
+ if (!*arg || !*(arg+1))
+ return;
+ arg += 1;
+ offset = strcspn(arg, ":");
+
+ ld->method = METHOD_HD;
+ ld->stage2Data = calloc(sizeof(struct hdInstallData *), 1);
+ ((struct hdInstallData *)ld->stage2Data)->partition = strndup(arg, offset);
+ arg += offset;
+ if (*arg && *(arg+1))
+ ((struct hdInstallData *)ld->stage2Data)->directory = strdup(arg+1);
+ else
+ ((struct hdInstallData *)ld->stage2Data)->directory = NULL;
}
- free(dup);
}
--
1.6.5.2

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

Hi,

On 01/15/2010 05:38 PM, Peter Jones wrote:

setStage2LocFromCmdline() doesn't need to do so much allocation just to
use strtok(), which should be cleansed with fire anyway. Get rid of
both.
---
loader/method.c | 68 +++++++++++++++++++++++++++---------------------------
1 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/loader/method.c b/loader/method.c
index 5f3a7ce..a49d01e 100644
--- a/loader/method.c
+++ b/loader/method.c
@@ -500,39 +500,39 @@ int getFileFromBlockDevice(char *device, char *path, char * dest) {
}

void setStage2LocFromCmdline(char * arg, struct loaderData_s * ld) {
- char * c, * dup;
-
- dup = strdup(arg);
- c = dup;
- /* : will let us delimit real information on the method */
- if ((c = strtok(c, ":"))) {
- c = strtok(NULL, ":");
-
- if (!strncmp(arg, "nfs:", 4)) {
- ld->method = METHOD_NFS;
- ld->stage2Data = calloc(sizeof(struct nfsInstallData *), 1);
-
- parseNfsHostPathOpts(arg + 4,
-&(((struct nfsInstallData *)ld->stage2Data)->host),
-&(((struct nfsInstallData *)ld->stage2Data)->directory),
-&(((struct nfsInstallData *)ld->stage2Data)->mountOpts));
- } else if (!strncmp(arg, "ftp:", 4) ||
- !strncmp(arg, "http", 4)) {
- ld->method = METHOD_URL;
- ld->stage2Data = calloc(sizeof(struct urlInstallData *), 1);
- ((urlInstallData *)ld->stage2Data)->url = strdup(arg);
- } else if (!strncmp(arg, "cdrom:", 6)) {
- ld->method = METHOD_CDROM;
- } else if (!strncmp(arg, "harddrive:", 10) ||
- !strncmp(arg, "hd:", 3)) {
- ld->method = METHOD_HD;
- ld->stage2Data = calloc(sizeof(struct hdInstallData *), 1);
- ((struct hdInstallData *)ld->stage2Data)->partition = strdup(c);
- if ((c = strtok(NULL, ":")))
- ((struct hdInstallData *)ld->stage2Data)->directory = strdup(c);
- else
- ((struct hdInstallData *)ld->stage2Data)->directory = NULL;
- }
+ if (!strncmp(arg, "nfs:", 4)) {
+ ld->method = METHOD_NFS;
+ ld->stage2Data = calloc(sizeof(struct nfsInstallData *), 1);
+
+ parseNfsHostPathOpts(arg + 4,
+&(((struct nfsInstallData *)ld->stage2Data)->host),
+&(((struct nfsInstallData *)ld->stage2Data)->directory),
+&(((struct nfsInstallData *)ld->stage2Data)->mountOpts));
+ } else if (!strncmp(arg, "ftp:", 4) ||
+ !strncmp(arg, "http", 4)) {
+ ld->method = METHOD_URL;
+ ld->stage2Data = calloc(sizeof(struct urlInstallData *), 1);
+ ((urlInstallData *)ld->stage2Data)->url = strdup(arg);
+ } else if (!strncmp(arg, "cdrom:", 6)) {
+ ld->method = METHOD_CDROM;
+ } else if (!strncmp(arg, "harddrive:", 10) ||
+ !strncmp(arg, "hd:", 3)) {
+ size_t offset;
+
+ offset = strcspn(arg, ":");


This setting of offset is unnecesary.


+ arg += strcspn(arg, ":");
+ if (!*arg || !*(arg+1))
+ return;
+ arg += 1;
+ offset = strcspn(arg, ":");
+
+ ld->method = METHOD_HD;
+ ld->stage2Data = calloc(sizeof(struct hdInstallData *), 1);
+ ((struct hdInstallData *)ld->stage2Data)->partition = strndup(arg, offset);
+ arg += offset;
+ if (*arg&& *(arg+1))
+ ((struct hdInstallData *)ld->stage2Data)->directory = strdup(arg+1);
+ else
+ ((struct hdInstallData *)ld->stage2Data)->directory = NULL;
}
- free(dup);
}


Otherwise ack.

Regards,

Hans

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

On 01/17/2010 08:52 AM, Hans de Goede wrote:
> Hi,
>
> On 01/15/2010 05:38 PM, Peter Jones wrote:
>> setStage2LocFromCmdline() doesn't need to do so much allocation just to
>> use strtok(), which should be cleansed with fire anyway. Get rid of
>> both.
>> ---
>> loader/method.c | 68
>> +++++++++++++++++++++++++++---------------------------
>> 1 files changed, 34 insertions(+), 34 deletions(-)
>>
>> diff --git a/loader/method.c b/loader/method.c
>> index 5f3a7ce..a49d01e 100644
>> --- a/loader/method.c
>> +++ b/loader/method.c
>> @@ -500,39 +500,39 @@ int getFileFromBlockDevice(char *device, char
>> *path, char * dest) {
>> }
>>
>> void setStage2LocFromCmdline(char * arg, struct loaderData_s * ld) {
>> - char * c, * dup;
>> -
>> - dup = strdup(arg);
>> - c = dup;
>> - /* : will let us delimit real information on the method */
>> - if ((c = strtok(c, ":"))) {
>> - c = strtok(NULL, ":");
>> -
>> - if (!strncmp(arg, "nfs:", 4)) {
>> - ld->method = METHOD_NFS;
>> - ld->stage2Data = calloc(sizeof(struct nfsInstallData *), 1);
>> -
>> - parseNfsHostPathOpts(arg + 4,
>> -&(((struct nfsInstallData *)ld->stage2Data)->host),
>> -&(((struct nfsInstallData *)ld->stage2Data)->directory),
>> -&(((struct nfsInstallData *)ld->stage2Data)->mountOpts));
>> - } else if (!strncmp(arg, "ftp:", 4) ||
>> - !strncmp(arg, "http", 4)) {
>> - ld->method = METHOD_URL;
>> - ld->stage2Data = calloc(sizeof(struct urlInstallData *), 1);
>> - ((urlInstallData *)ld->stage2Data)->url = strdup(arg);
>> - } else if (!strncmp(arg, "cdrom:", 6)) {
>> - ld->method = METHOD_CDROM;
>> - } else if (!strncmp(arg, "harddrive:", 10) ||
>> - !strncmp(arg, "hd:", 3)) {
>> - ld->method = METHOD_HD;
>> - ld->stage2Data = calloc(sizeof(struct hdInstallData *), 1);
>> - ((struct hdInstallData *)ld->stage2Data)->partition =
>> strdup(c);
>> - if ((c = strtok(NULL, ":")))
>> - ((struct hdInstallData *)ld->stage2Data)->directory =
>> strdup(c);
>> - else
>> - ((struct hdInstallData *)ld->stage2Data)->directory =
>> NULL;
>> - }
>> + if (!strncmp(arg, "nfs:", 4)) {
>> + ld->method = METHOD_NFS;
>> + ld->stage2Data = calloc(sizeof(struct nfsInstallData *), 1);
>> +
>> + parseNfsHostPathOpts(arg + 4,
>> +&(((struct nfsInstallData *)ld->stage2Data)->host),
>> +&(((struct nfsInstallData *)ld->stage2Data)->directory),
>> +&(((struct nfsInstallData *)ld->stage2Data)->mountOpts));
>> + } else if (!strncmp(arg, "ftp:", 4) ||
>> + !strncmp(arg, "http", 4)) {
>> + ld->method = METHOD_URL;
>> + ld->stage2Data = calloc(sizeof(struct urlInstallData *), 1);
>> + ((urlInstallData *)ld->stage2Data)->url = strdup(arg);
>> + } else if (!strncmp(arg, "cdrom:", 6)) {
>> + ld->method = METHOD_CDROM;
>> + } else if (!strncmp(arg, "harddrive:", 10) ||
>> + !strncmp(arg, "hd:", 3)) {
>> + size_t offset;
>> +
>> + offset = strcspn(arg, ":");
>
> This setting of offset is unnecesary.

Indeed; fixed and pushed. I also pushed a patch to remove the trailing
whitespace on the "ftp:" strncmp line above.

--
Peter

All parts should go together without forcing. You must remember that
the parts you are reassembling were disassembled by you. Therefore,
if you can't get them together again, there must be a reason. By all
means, do not use a hammer.
-- IBM maintenance manual, 1925

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list