Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.c > #382803 > unrolled thread

getOpsFromSelf() (Long Post)

Back to article view | Back to comp.lang.c

Contents

  getOpsFromSelf() (Long Post) porkchop@invalid.foo (Mike Sanders) - 2024-02-20 06:56 +0000
    Re: getOpsFromSelf() (Long Post) Spiros Bousbouras <spibou@gmail.com> - 2024-02-24 16:19 +0000
      Re: getOpsFromSelf() (Long Post) Spiros Bousbouras <spibou@gmail.com> - 2024-02-24 17:06 +0000
        Re: getOpsFromSelf() (Long Post) porkchop@invalid.foo (Mike Sanders) - 2024-02-24 21:25 +0000
          Re: getOpsFromSelf() (Long Post) Keith Thompson <Keith.S.Thompson+u@gmail.com> - 2024-02-24 17:59 -0800
            Re: getOpsFromSelf() (Long Post) porkchop@invalid.foo (Mike Sanders) - 2024-02-25 11:49 +0000
              Re: getOpsFromSelf() (Long Post) Keith Thompson <Keith.S.Thompson+u@gmail.com> - 2024-02-25 14:54 -0800
                Re: getOpsFromSelf() (Long Post) Lew Pitcher <lew.pitcher@digitalfreehold.ca> - 2024-02-25 22:58 +0000
                Re: getOpsFromSelf() (Long Post) dave_thompson_2@comcast.net - 2024-03-01 18:37 -0500
                  Re: getOpsFromSelf() (Long Post) porkchop@invalid.foo (Mike Sanders) - 2024-03-02 01:10 +0000
                    Re: getOpsFromSelf() (Long Post) Lew Pitcher <lew.pitcher@digitalfreehold.ca> - 2024-03-02 02:36 +0000
                      Re: getOpsFromSelf() (Long Post) porkchop@invalid.foo (Mike Sanders) - 2024-03-02 12:13 +0000
          Re: getOpsFromSelf() (Long Post) Spiros Bousbouras <spibou@gmail.com> - 2024-02-26 15:07 +0000
            Re: getOpsFromSelf() (Long Post) porkchop@invalid.foo (Mike Sanders) - 2024-03-02 12:16 +0000
      Re: getOpsFromSelf() (Long Post) porkchop@invalid.foo (Mike Sanders) - 2024-02-24 21:22 +0000
        Re: getOpsFromSelf() (Long Post) Ben Bacarisse <ben.usenet@bsb.me.uk> - 2024-02-25 00:48 +0000
          Re: getOpsFromSelf() (Long Post) porkchop@invalid.foo (Mike Sanders) - 2024-02-25 01:51 +0000
        Re: getOpsFromSelf() (Long Post) Spiros Bousbouras <spibou@gmail.com> - 2024-02-26 14:50 +0000
          Re: getOpsFromSelf() (Long Post) porkchop@invalid.foo (Mike Sanders) - 2024-02-26 15:15 +0000
          Re: getOpsFromSelf() (Long Post) Keith Thompson <Keith.S.Thompson+u@gmail.com> - 2024-02-26 13:38 -0800
          Re: getOpsFromSelf() (Long Post) Spiros Bousbouras <spibou@gmail.com> - 2024-02-26 21:40 +0000
          Re: getOpsFromSelf() (Long Post) scott@slp53.sl.home (Scott Lurndal) - 2024-02-27 16:17 +0000
          Re: getOpsFromSelf() (Long Post) Spiros Bousbouras <spibou@gmail.com> - 2024-02-27 17:04 +0000
          Re: getOpsFromSelf() (Long Post) David Brown <david.brown@hesbynett.no> - 2024-02-27 18:20 +0100
      Re: getOpsFromSelf() (Long Post) scott@slp53.sl.home (Scott Lurndal) - 2024-02-25 18:42 +0000
        Re: getOpsFromSelf() (Long Post) porkchop@invalid.foo (Mike Sanders) - 2024-02-26 15:20 +0000
    Re: getOpsFromSelf() (Long Post) immibis <news@immibis.com> - 2024-02-28 22:21 +0100
      Re: getOpsFromSelf() (Long Post) porkchop@invalid.foo (Mike Sanders) - 2024-03-01 00:38 +0000

Page 1 of 2  [1] 2  Next page →

#382803 — getOpsFromSelf() (Long Post)

Just thinking aloud... Imagining here allowing more advanced users of
my app to modify a string of digits residing at the end of my project's
binary/exe. When the app loads, it reads in those digits. So far (knock
on wood), I've not run up against a single case where the code below has
failed to work under multiple OSs. A method mostly for power user's.

The user simply opens the compiled project in their favorite hex editor
& modifies the pre-existing string, a pseudo bitmask of sorts. Certainly
the user could unwittingly add/remove bytes rather than only modifying the
bytes, still its a nifty idea. But practical? I'm not so sure on that point.

Any other gotchas of note?

int main(int argc, char *argv[]) {

#ifdef WIN
    getOpsFromSelf(NULL);
#else
    getOpsFromSelf(argv[0]);
#endif

    // work

    return 0;

}

void getOpsFromSelf(char *fname) {

#ifdef WIN
    char path[MAX_PATH];
    GetModuleFileName(NULL, path, MAX_PATH);
#else
    char path[PATH_MAX];
    char *tmp = getpath(fname);
    strncpy(path, tmp, sizeof(path));
    path[sizeof(path) - 1] = '\0';
    free(tmp);
#endif

    FILE *bin = fopen(path, "rb");
    if (bin == NULL) return; // return if file cant be opened

    fseek(bin, -3, SEEK_END); // jump 3 bytes before end of file
    unsigned char bytes[3];
    fread(bytes, 1, 3, bin);
    fclose(bin);

    // ensure last 3 bytes are digits, else return
    for (int i = 0; i < 2; i++) if (!isdigit(bytes[i])) return;

    // assign values from bytes
    ops.x = (bytes[0] == '1' || bytes[0] == '2') ? bytes[0] - '0' : 1; // 1-2
    ops.y = (bytes[1] == '1') ? bytes[1] - '0' : 0;                    // 0-1
    ops.z = (bytes[2] == '1') ? bytes[2] - '0' : 0;                    // 0-1

}

#ifdef NIX

char *getpath(char *fname) {

    const char *PATH_SEPARATOR = ":"; // unix-like
    const char *DIR_SEPARATOR = "/";  // unix-like
    const int MAX_SIZE = 1024;

    struct stat buffer;

    if (stat(fname, &buffer) == 0) {
        char *result = strdup(fname);
        if (!result) return NULL; // memory allocation error <--
        return result;
    }

    char *path_env = getenv("PATH");
    if (!path_env) return NULL;

    char path_env_copy[MAX_SIZE];
    strncpy(path_env_copy, path_env, sizeof(path_env_copy));
    path_env_copy[sizeof(path_env_copy) - 1] = '\0';

    char full_path[MAX_SIZE];
    char *dir = strtok(path_env_copy, PATH_SEPARATOR);

    while (dir) {
        snprintf(full_path, sizeof(full_path), "%s%s%s", dir, 
            (dir[strlen(dir) - 1] == DIR_SEPARATOR[0] ? "" :
                DIR_SEPARATOR), fname);
        if (stat(full_path, &buffer) == 0) {
            char *result = strdup(full_path);
            if (!result) return NULL; // memory allocation error <--
            return result;
        }
        dir = strtok(NULL, PATH_SEPARATOR);
    }

    return NULL;
}

#endif

-- 
:wq
Mike Sanders

[toc] | [next] | [standalone]

#382972

On Tue, 20 Feb 2024 06:56:06 -0000 (UTC)
porkchop@invalid.foo (Mike Sanders) wrote:
> 
> Just thinking aloud... Imagining here allowing more advanced users of
> my app to modify a string of digits residing at the end of my project's
> binary/exe. When the app loads, it reads in those digits. So far (knock
> on wood), I've not run up against a single case where the code below has
> failed to work under multiple OSs. A method mostly for power user's.
> 
> The user simply opens the compiled project in their favorite hex editor
> & modifies the pre-existing string, a pseudo bitmask of sorts. Certainly
> the user could unwittingly add/remove bytes rather than only modifying the
> bytes, still its a nifty idea. But practical? I'm not so sure on that point.

Compared to other options like command line options or configuration files or
environmental variables or asking the user interactively to set the values ,
this approach seems very much worse. It is a lot less practical *and* fragile.
How can you know that this specific string will exist at a certain position
in the executable ?

> Any other gotchas of note?
> 
> int main(int argc, char *argv[]) {
> 
> #ifdef WIN
>     getOpsFromSelf(NULL);
> #else
>     getOpsFromSelf(argv[0]);

There are Unix conventions on what  argv[0]  will contain but no guarantees.
In particular , there is no guaranteed way to get the pathname of the
executable from  argv[0] .This is another way your approach is fragile.

> #endif
> 
>     // work
> 
>     return 0;
> 
> }
> 
> void getOpsFromSelf(char *fname) {
> 
> #ifdef WIN
>     char path[MAX_PATH];
>     GetModuleFileName(NULL, path, MAX_PATH);
> #else
>     char path[PATH_MAX];
>     char *tmp = getpath(fname);
>     strncpy(path, tmp, sizeof(path));

What happens if  getpath()  returns NULL ?

>     path[sizeof(path) - 1] = '\0';
>     free(tmp);
> #endif
> 
>     FILE *bin = fopen(path, "rb");
>     if (bin == NULL) return; // return if file cant be opened
> 
>     fseek(bin, -3, SEEK_END); // jump 3 bytes before end of file
>     unsigned char bytes[3];
>     fread(bytes, 1, 3, bin);
>     fclose(bin);

You keep writing 3 .Best to use a symbolic constant.

> 
>     // ensure last 3 bytes are digits, else return
>     for (int i = 0; i < 2; i++) if (!isdigit(bytes[i])) return;
> 
>     // assign values from bytes
>     ops.x = (bytes[0] == '1' || bytes[0] == '2') ? bytes[0] - '0' : 1; // 1-2
>     ops.y = (bytes[1] == '1') ? bytes[1] - '0' : 0;                    // 0-1
>     ops.z = (bytes[2] == '1') ? bytes[2] - '0' : 0;                    // 0-1

I take it  ops  is a global variable ?

> 
> }
> 
> #ifdef NIX
> 
> char *getpath(char *fname) {
> 
>     const char *PATH_SEPARATOR = ":"; // unix-like
>     const char *DIR_SEPARATOR = "/";  // unix-like
>     const int MAX_SIZE = 1024;
> 
>     struct stat buffer;
> 
>     if (stat(fname, &buffer) == 0) {
>         char *result = strdup(fname);
>         if (!result) return NULL; // memory allocation error <--
>         return result;
>     }

On Unix , if you just want to test for the existence of a file , access()
is a simpler way to do it.

>     char *path_env = getenv("PATH");
>     if (!path_env) return NULL;
> 
>     char path_env_copy[MAX_SIZE];
>     strncpy(path_env_copy, path_env, sizeof(path_env_copy));
>     path_env_copy[sizeof(path_env_copy) - 1] = '\0';
> 
>     char full_path[MAX_SIZE];
>     char *dir = strtok(path_env_copy, PATH_SEPARATOR);
> 
>     while (dir) {
>         snprintf(full_path, sizeof(full_path), "%s%s%s", dir, 
>             (dir[strlen(dir) - 1] == DIR_SEPARATOR[0] ? "" :
>                 DIR_SEPARATOR), fname);
>         if (stat(full_path, &buffer) == 0) {
>             char *result = strdup(full_path);
>             if (!result) return NULL; // memory allocation error <--
This line is redundant.

>             return result;
>         }
>         dir = strtok(NULL, PATH_SEPARATOR);
>     }
> 
>     return NULL;
> }

I wouldn't use  strtok()  at all for this but rather  strchr()  and keep
track myself of the position in the string and not modify the string at
all. This way

- you can use the string returned by  getenv()  directly instead of having
  to copy it.

- you don't have to call  strlen()  when you have already traversed the
  string to find the separator. Traversing the string for a second time
  probably won't have a noticeable effect for performance in this context
  but this kind of thing where the code has done a certain somewhat costly
  computation (traversing part of the string) and then immediately does
  again almost the same computation instead of using the result found by
  the first computation , grates me.

> 
> #endif

-- 
- Yep 100x better then US prison, no pillows, no tv, no nothing except
  a blanket and a small pad to sleep on.
- It is not a fair comparison. You have to look at it from a racist
  perspective. Swedish prisons are meant for white folks. American
  prisons are not.
http://suprbay.org/showthread.php?t=51513&page=2

[toc] | [prev] | [next] | [standalone]

#382973

On Sat, 24 Feb 2024 16:19:11 -0000 (UTC)
Spiros Bousbouras <spibou@gmail.com> wrote:
> On Tue, 20 Feb 2024 06:56:06 -0000 (UTC)
> porkchop@invalid.foo (Mike Sanders) wrote:
> >     char *path_env = getenv("PATH");
> >     if (!path_env) return NULL;
> > 
> >     char path_env_copy[MAX_SIZE];
> >     strncpy(path_env_copy, path_env, sizeof(path_env_copy));
> >     path_env_copy[sizeof(path_env_copy) - 1] = '\0';
> > 
> >     char full_path[MAX_SIZE];
> >     char *dir = strtok(path_env_copy, PATH_SEPARATOR);
> > 
> >     while (dir) {
> >         snprintf(full_path, sizeof(full_path), "%s%s%s", dir, 
> >             (dir[strlen(dir) - 1] == DIR_SEPARATOR[0] ? "" :
> >                 DIR_SEPARATOR), fname);
> >         if (stat(full_path, &buffer) == 0) {
> >             char *result = strdup(full_path);
> >             if (!result) return NULL; // memory allocation error <--
> This line is redundant.
> 
> >             return result;
> >         }
> >         dir = strtok(NULL, PATH_SEPARATOR);

If I remeber correctly , the convention is that if  ::  exists in PATH
then it's the same as  :.:  i.e. the current working directory. Your
code does not handle this case. Yet another reason to do the searching
in the string yourself instead of using  strtok() .

> >     }
> > 
> >     return NULL;
> > }
> 
> I wouldn't use  strtok()  at all for this but rather  strchr()  and keep
> track myself of the position in the string and not modify the string at
> all. This way
> 
> - you can use the string returned by  getenv()  directly instead of having
>   to copy it.
> 
> - you don't have to call  strlen()  when you have already traversed the
>   string to find the separator. Traversing the string for a second time
>   probably won't have a noticeable effect for performance in this context
>   but this kind of thing where the code has done a certain somewhat costly
>   computation (traversing part of the string) and then immediately does
>   again almost the same computation instead of using the result found by
>   the first computation , grates me.
> 
> > 
> > #endif

-- 
God grant me serenity to accept the code I cannot change, courage to
change the code I can, and wisdom to know the difference.
  Erik Naggum

[toc] | [prev] | [next] | [standalone]

#382978

Spiros Bousbouras <spibou@gmail.com> wrote:

> If I remeber correctly , the convention is that if  ::  exists in PATH
> then it's the same as  :.:  i.e. the current working directory. Your
> code does not handle this case. Yet another reason to do the searching
> in the string yourself instead of using  strtok() .

It does handle the current directory... before it traverses the users path.

-- 
:wq
Mike Sanders

[toc] | [prev] | [next] | [standalone]

#382993

porkchop@invalid.foo (Mike Sanders) writes:
> Spiros Bousbouras <spibou@gmail.com> wrote:
>> If I remeber correctly , the convention is that if  ::  exists in PATH
>> then it's the same as  :.:  i.e. the current working directory. Your
>> code does not handle this case. Yet another reason to do the searching
>> in the string yourself instead of using  strtok() .
>
> It does handle the current directory... before it traverses the users path.

I haven't read the code closely enough to know just what it does, but I
see it examines the $PATH environment variable.

(This is specific to Unix-like systems.)

If your code looks at the current directory regardless of what's in
$PATH, then it's probably misbehaving.  If the current directory isn't
specified in $PATH, the current directory shouldn't be searched.

$PATH is a list of directory paths, separated by ':' characters.  An
empty field refers to the current directory, which can also be specified
as '.'.  This can appear at the beginning (if PATH starts with ":"), at
the end (if it ends with ":", or in the middle (if it contains ":").

It's recommended *not* to include the current directory in $PATH.

-- 
Keith Thompson (The_Other_Keith) Keith.S.Thompson+u@gmail.com
Working, but not speaking, for Medtronic
void Void(void) { Void(); } /* The recursive call of the void */

[toc] | [prev] | [next] | [standalone]

#383004

Keith Thompson <Keith.S.Thompson+u@gmail.com> wrote:

> It's recommended *not* to include the current directory in $PATH.

I've been thinking about that very thing in fact. It renders my
idea of a binary reading a few bytes from its-self all but dead.
I wont subvert a given security model for a configuration flag
or two...

But there you have it, a wonky idea, I've got plenty of them.

-- 
:wq
Mike Sanders

[toc] | [prev] | [next] | [standalone]

#383020

porkchop@invalid.foo (Mike Sanders) writes:
> Keith Thompson <Keith.S.Thompson+u@gmail.com> wrote:
>> It's recommended *not* to include the current directory in $PATH.
>
> I've been thinking about that very thing in fact. It renders my
> idea of a binary reading a few bytes from its-self all but dead.
> I wont subvert a given security model for a configuration flag
> or two...
>
> But there you have it, a wonky idea, I've got plenty of them.

Under Linux, a program can find its executable via /proc/$$/exe .

-- 
Keith Thompson (The_Other_Keith) Keith.S.Thompson+u@gmail.com
Working, but not speaking, for Medtronic
void Void(void) { Void(); } /* The recursive call of the void */

[toc] | [prev] | [next] | [standalone]

#383021

On Sun, 25 Feb 2024 14:54:35 -0800, Keith Thompson wrote:

> porkchop@invalid.foo (Mike Sanders) writes:
>> Keith Thompson <Keith.S.Thompson+u@gmail.com> wrote:
>>> It's recommended *not* to include the current directory in $PATH.
>>
>> I've been thinking about that very thing in fact. It renders my
>> idea of a binary reading a few bytes from its-self all but dead.
>> I wont subvert a given security model for a configuration flag
>> or two...
>>
>> But there you have it, a wonky idea, I've got plenty of them.
> 
> Under Linux, a program can find its executable via /proc/$$/exe .

With the caveat that /proc/$$/exe is a symlink.
As such, it may participate in a race condition where, if the
"real" executable (the target of the symlink) is deleted or
moved before the program references it's /proc/$$/exe, the
symlink may point to a non-existent, or /different/ executable. 


-- 
Lew Pitcher
"In Skills We Trust"

[toc] | [prev] | [next] | [standalone]

#383234

On Sun, 25 Feb 2024 14:54:35 -0800, Keith Thompson
<Keith.S.Thompson+u@gmail.com> wrote:

> porkchop@invalid.foo (Mike Sanders) writes:
> > Keith Thompson <Keith.S.Thompson+u@gmail.com> wrote:
> >> It's recommended *not* to include the current directory in $PATH.
> >
> > I've been thinking about that very thing in fact. It renders my
> > idea of a binary reading a few bytes from its-self all but dead.
> > I wont subvert a given security model for a configuration flag
> > or two...
> >
> > But there you have it, a wonky idea, I've got plenty of them.
> 
> Under Linux, a program can find its executable via /proc/$$/exe .

YM sprintf(buf, "/proc/%d/exe", getpid()) // or if paranoid snprintf
because substituting for $$ is a feature of shell and perl only. But
/proc/self/exe works in anything.

[toc] | [prev] | [next] | [standalone]

#383236

dave_thompson_2@comcast.net wrote:

> YM sprintf(buf, "/proc/%d/exe", getpid()) // or if paranoid snprintf
> because substituting for $$ is a feature of shell and perl only. But
> /proc/self/exe works in anything.

For Linux, sure. But the BSDs & Apple & Win...

Reading itsself, its real self, seems to me to be the only
cross platform way.

-- 
:wq
Mike Sanders

[toc] | [prev] | [next] | [standalone]

#383238

On Sat, 02 Mar 2024 01:10:09 +0000, Mike Sanders wrote:

> dave_thompson_2@comcast.net wrote:
> 
>> YM sprintf(buf, "/proc/%d/exe", getpid()) // or if paranoid snprintf
>> because substituting for $$ is a feature of shell and perl only. But
>> /proc/self/exe works in anything.
> 
> For Linux, sure. But the BSDs & Apple & Win...
> 
> Reading itsself, its real self, seems to me to be the only
> cross platform way.

Perhaps, but there seems to be no cross-platform way for a program
to determine the "itself" to read. 


-- 
Lew Pitcher
"In Skills We Trust"

[toc] | [prev] | [next] | [standalone]

#383240

Lew Pitcher <lew.pitcher@digitalfreehold.ca> wrote:

>> For Linux, sure. But the BSDs & Apple & Win...
>> 
>> Reading itsself, its real self, seems to me to be the only
>> cross platform way.
> 
> Perhaps, but there seems to be no cross-platform way for a program
> to determine the "itself" to read. 

Another thing too Lew... what if there is *no* path (for instance
something like a small controller/embedded environment)? It couldn't
find itsself in any event then I'm thinking.

-- 
:wq
Mike Sanders

[toc] | [prev] | [next] | [standalone]

#383038

On Sat, 24 Feb 2024 21:25:56 -0000 (UTC)
porkchop@invalid.foo (Mike Sanders) wrote:
> Spiros Bousbouras <spibou@gmail.com> wrote:
> 
> > If I remeber correctly , the convention is that if  ::  exists in PATH
> > then it's the same as  :.:  i.e. the current working directory. Your
> > code does not handle this case. Yet another reason to do the searching
> > in the string yourself instead of using  strtok() .
> 
> It does handle the current directory... before it traverses the users path.

Which may be the wrong order. Regardless of the overall merits of your idea ,
it should mimic as close as possible how the system finds executables. This
means not search in the current directory unless the appropriate constructs
exist in PATH and search the components in the same order as they appear in
PATH .This implies recognising  ::  in PATH and my point is that using
strtok() (apart from its other issues) makes it harder to do this.

Replying to  <urf9g5$1qtds$1@dont-email.me> :

> Keith Thompson <Keith.S.Thompson+u@gmail.com> wrote:
> > It's recommended *not* to include the current directory in $PATH.
> 
> I've been thinking about that very thing in fact. It renders my
> idea of a binary reading a few bytes from its-self all but dead.
> I wont subvert a given security model for a configuration flag
> or two...

It is fine to include the current directory in PATH as long as you do it
right. Details are not on topic for this group.

-- 
vlaho.ninja/menu

[toc] | [prev] | [next] | [standalone]

#383241

Spiros Bousbouras <spibou@gmail.com> wrote:

> Which may be the wrong order. Regardless of the overall merits of your idea ,
> it should mimic as close as possible how the system finds executables. This
> means not search in the current directory unless the appropriate constructs
> exist in PATH and search the components in the same order as they appear in
> PATH .This implies recognising  ::  in PATH and my point is that using
> strtok() (apart from its other issues) makes it harder to do this.

Yes, sounds like good advice to me Spiros.

-- 
:wq
Mike Sanders

[toc] | [prev] | [next] | [standalone]

#382977

Spiros Bousbouras <spibou@gmail.com> wrote:

> Compared to other options like command line options or configuration files or
> environmental variables or asking the user interactively to set the values ,
> this approach seems very much worse. It is a lot less practical *and* fragile.
> How can you know that this specific string will exist at a certain position
> in the executable ?

Because it checks that does...

>>     // ensure last 3 bytes are digits, else return
>>     for (int i = 0; i < 2; i++) if (!isdigit(bytes[i])) return;
 
> What happens if  getpath()  returns NULL ?

Ouch busted...
 
> You keep writing 3 .Best to use a symbolic constant.

Why? 3 is always 3. Maybe I dont undestand what you mean...
 
> I take it  ops  is a global variable ?

In this example, yes.

> On Unix , if you just want to test for the existence of a file , access()
> is a simpler way to do it.
> 
>>     char *path_env = getenv("PATH");
>>     if (!path_env) return NULL;
>> 
>>     char path_env_copy[MAX_SIZE];
>>     strncpy(path_env_copy, path_env, sizeof(path_env_copy));
>>     path_env_copy[sizeof(path_env_copy) - 1] = '\0';
>> 
>>     char full_path[MAX_SIZE];
>>     char *dir = strtok(path_env_copy, PATH_SEPARATOR);
>> 
>>     while (dir) {
>>         snprintf(full_path, sizeof(full_path), "%s%s%s", dir, 
>>             (dir[strlen(dir) - 1] == DIR_SEPARATOR[0] ? "" :
>>                 DIR_SEPARATOR), fname);
>>         if (stat(full_path, &buffer) == 0) {
>>             char *result = strdup(full_path);
>>             if (!result) return NULL; // memory allocation error <--

> This line is redundant.

Please explain why you think so.

> I wouldn't use  strtok()  at all for this but rather  strchr()  and keep
> track myself of the position in the string and not modify the string at
> all. This way
> 
> - you can use the string returned by  getenv()  directly instead of having
>   to copy it.
> 
> - you don't have to call  strlen()  when you have already traversed the
>   string to find the separator. Traversing the string for a second time
>   probably won't have a noticeable effect for performance in this context
>   but this kind of thing where the code has done a certain somewhat costly
>   computation (traversing part of the string) and then immediately does
>   again almost the same computation instead of using the result found by
>   the first computation , grates me.

I dunno, is it perfect? Nah. Is it a good start? Well, yeah =)

-- 
:wq
Mike Sanders

[toc] | [prev] | [next] | [standalone]

#382988

porkchop@invalid.foo (Mike Sanders) writes:

> Spiros Bousbouras <spibou@gmail.com> wrote:
>
>> Compared to other options like command line options or configuration files or
>> environmental variables or asking the user interactively to set the values ,
>> this approach seems very much worse. It is a lot less practical *and* fragile.
>> How can you know that this specific string will exist at a certain position
>> in the executable ?
>
> Because it checks that does...
>
>>>     // ensure last 3 bytes are digits, else return
>>>     for (int i = 0; i < 2; i++) if (!isdigit(bytes[i])) return;
>  
>> What happens if  getpath()  returns NULL ?
>
> Ouch busted...
>  
>> You keep writing 3 .Best to use a symbolic constant.
>
> Why? 3 is always 3. Maybe I dont undestand what you mean...

And 3 != 2.  Maybe using "CODE_LENGTH" everywhere would have prevented
the (probable) bug above where the loop tests only two characters.
After

  #define CODE_LENGTH 3

would you have written

  for (int i = 0; i < CODE_LENGTH - 1; i++) ...

?  I don't know, but I think you might have thought twice about that -1.

>> I take it  ops  is a global variable ?
>
> In this example, yes.
>
>> On Unix , if you just want to test for the existence of a file , access()
>> is a simpler way to do it.
>> 
>>>     char *path_env = getenv("PATH");
>>>     if (!path_env) return NULL;
>>> 
>>>     char path_env_copy[MAX_SIZE];
>>>     strncpy(path_env_copy, path_env, sizeof(path_env_copy));
>>>     path_env_copy[sizeof(path_env_copy) - 1] = '\0';
>>> 
>>>     char full_path[MAX_SIZE];
>>>     char *dir = strtok(path_env_copy, PATH_SEPARATOR);
>>> 
>>>     while (dir) {
>>>         snprintf(full_path, sizeof(full_path), "%s%s%s", dir, 
>>>             (dir[strlen(dir) - 1] == DIR_SEPARATOR[0] ? "" :
>>>                 DIR_SEPARATOR), fname);
>>>         if (stat(full_path, &buffer) == 0) {
>>>             char *result = strdup(full_path);
>>>             if (!result) return NULL; // memory allocation error <--
>
>> This line is redundant.
>
> Please explain why you think so.

Given the following line (return result;) there is no point in returning
NULL is the special case where result is NULL since NULL will be
returned when result is NULL.  In fact, the whole three-line compound
statement is (semantically) equivalent to

  return strdup(full_path);

>> I wouldn't use  strtok()  at all for this but rather  strchr()  and keep
>> track myself of the position in the string and not modify the string at
>> all. This way
>> 
>> - you can use the string returned by  getenv()  directly instead of having
>>   to copy it.
>> 
>> - you don't have to call  strlen()  when you have already traversed the
>>   string to find the separator. Traversing the string for a second time
>>   probably won't have a noticeable effect for performance in this context
>>   but this kind of thing where the code has done a certain somewhat costly
>>   computation (traversing part of the string) and then immediately does
>>   again almost the same computation instead of using the result found by
>>   the first computation , grates me.
>
> I dunno, is it perfect? Nah. Is it a good start? Well, yeah =)

I could not tell what it was supposed to do so I didn't comment, but
there's definitely a "let's just try" sense to the code.  For example,
you examine and use three characters, but you don't test if three
characters were actually read.

One specific C issue did jump out at me:

  "A binary stream need not meaningfully support fseek calls with a
  whence value of SEEK_END."

-- 
Ben.

[toc] | [prev] | [next] | [standalone]

#382992

Ben Bacarisse <ben.usenet@bsb.me.uk> wrote:

> ?  I don't know, but I think you might have thought twice about that -1.

Aye, good point.

> I could not tell what it was supposed to do so I didn't comment, but
> there's definitely a "let's just try" sense to the code.  For example,
> you examine and use three characters, but you don't test if three
> characters were actually read.

Well, that's all that it is though Ben, the development of an idea.
(Thinking aloud was term I used), maybe I'll whittle on it more
sometime, maybe you can tweak it too ya'know, post some code,
live life, be happy Mr. Crabby =)

-- 
:wq
Mike Sanders

[toc] | [prev] | [next] | [standalone]

#383037

On Sat, 24 Feb 2024 21:22:55 -0000 (UTC)
porkchop@invalid.foo (Mike Sanders) wrote:
> Spiros Bousbouras <spibou@gmail.com> wrote:
> 
> > Compared to other options like command line options or configuration files or
> > environmental variables or asking the user interactively to set the values ,
> > this approach seems very much worse. It is a lot less practical *and* fragile.
> > How can you know that this specific string will exist at a certain position
> > in the executable ?
> 
> Because it checks that does...

Who checks it and using what algorithm ?

> >>     // ensure last 3 bytes are digits, else return
> >>     for (int i = 0; i < 2; i++) if (!isdigit(bytes[i])) return;
>  
> > What happens if  getpath()  returns NULL ?
> 
> Ouch busted...
>  
> > You keep writing 3 .Best to use a symbolic constant.
> 
> Why? 3 is always 3. Maybe I dont undestand what you mean...

For example , your code has

     fseek(bin, -3, SEEK_END); // jump 3 bytes before end of file
     unsigned char bytes[3];
     fread(bytes, 1, 3, bin);

.Just reading this it is not immediate clear whether all appearances of  3
refer to the same conceptual entity or different ones which all happen to
have the value 3. Using a constant would have made it clear that it is the
former. Additionally , if you ever want to change the value , it is simpler
to only do it in one place.

[toc] | [prev] | [next] | [standalone]

#383040

Spiros Bousbouras <spibou@gmail.com> wrote:

> 
> For example , your code has
> 
>      fseek(bin, -3, SEEK_END); // jump 3 bytes before end of file
>      unsigned char bytes[3];
>      fread(bytes, 1, 3, bin);
> 
> .Just reading this it is not immediate clear whether all appearances of  3
> refer to the same conceptual entity or different ones which all happen to
> have the value 3. Using a constant would have made it clear that it is the
> former. Additionally , if you ever want to change the value , it is simpler
> to only do it in one place.

I want to contest this point since its scope is always/only within that
function, but... yes you are correct in the final analysis.

-- 
:wq
Mike Sanders

[toc] | [prev] | [next] | [standalone]

#383067

ram@zedat.fu-berlin.de (Stefan Ram) writes:
> Spiros Bousbouras <spibou@gmail.com> writes:
>>fseek(bin, -3, SEEK_END); // jump 3 bytes before end of file
>
>   A binary stream need not meaningfully support fseek calls
>   with a whence value of SEEK_END.

True, so the call is not completely portable -- but I doubt that there
are any hosted implementations that don't support such calls.

The reason for that line in the standard is to permit implementations
where, for example, binary files are represented as a sequence of
fixed-size blocks, with no information about how many bytes of the last
block are actually used.  The last block might be padded with zero
bytes.

POSIX and Windows are not the whole world, but both support binary files
that are arbitrary byte streams.

-- 
Keith Thompson (The_Other_Keith) Keith.S.Thompson+u@gmail.com
Working, but not speaking, for Medtronic
void Void(void) { Void(); } /* The recursive call of the void */

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | comp.lang.c

csiph-web