Time Implementation

[ctpham6]

Time
This branch aims to implement the Time program from the original Linux OS to FogOS. Time aims to measure the time it takes for other programs to run.

Usage:
time [program here] [other arguments]
Examples:

• time ls
• time ploop 1550000000
• time mkdir myfolder

Flag
-p : Prints out the time in POSIX format

Output:
Explanation:
It should print out the real, user, and sys time in that order. Real time (real) is the total elapsed time that passed since running your program of choice. It includes bits of time that the CPU does not capture. User space time (user) is the total amount of time the CPU spends in the user space. Kernel space time (sys) is the total amount fo time the CPU spends in the kernel space.

Default:
real 0m0.000s
user 0m0.000s
sys 0m0.000s

POSIX (-p):
real 0.00
user 0.00
sys 0.00

Note:
Kernel Space Time + User Space Time does NOT equate to Real Time and, by extension, any equivalent algebraic expressions of that is also not true

Tests
So you're looking to code review this. Awesome. I need the help
You can start by doing basic commands like:

• time (Should give 0's across the board)
• time -p (Should give 0's across the board)
• time ls (Compare it with C's time ls. ls in the OS shold be slower)
• time mkdir

PLOOP
ploop.c is another utility that I added that just does a for loop.

Usage
ploop [number]

This argument sets the amount of times the program loops

To save you the trouble of a number too big, 1550000000 gives around 5 seconds. You can go ahead and tweak that number to your liking

In Conclusion
time ploop 1550000000 is the example usage case

Have fun!

[HadyTinawi]

Verification

Issue:
“For my project, I want to implement a time utility that reports real, user, and sys time. It should also support a POSIX -p flag that outputs total seconds with two decimals.”

The time utility mostly meets the requirements of the issue. It spawns the command, records the time, and prints all three categories. However, two things stand out that should be addressed before merge.

Testing

After reviewing, the program does run and print timing information as expected. The -p flag switches formatting, and normal usage works with different commands. But, long durations may overflow the current type, and the -p branch does not currently conform to POSIX output (it prints seconds modulo 60 instead of total seconds with two decimals).

Code Walkthrough

Great stuff y’all. Two quick things:

1)Duration type can overflow int:
printtime takes int raw but an average duration can exceed 32-bit range (and you already compute with 64-bit elsewhere). You could switch to an unsigned 64-bit type throughout the timing path and use matching printf specifiers like I suggest below (or you could ignore everything I said which is what I would’ve done if I were you).

In user/time.c line 4: raw can exceed an int for longer runs. Also noticed that it treats this as milliseconds elsewhere. I'd suggest uint64_t raw_ms and do all math with uint64_t to avoid truncation.

2)POSIX -p format should show total seconds with two decimals:
The current -p branch derives seconds modulo 60 and builds a SS.cc string from milliseconds. POSIX expects total seconds (not modulo 60), with exactly two decimal digits. Example:

real 61.23s

for 61.23 seconds. I had ChatGPT suggest an alternate printf you can reference here: (also in user/time.c)
printf("%s\t%.2fs\n", category, raw_ms / 1000.0);

Hope this makes sense, reach out if you need further explanation!

Formatting

The code formatting is generally consistent and matches the style of other user-space utilities in the project. No major issues there.

High Level Checks:

[x]PR fully resolves the task/issue

[x] Does the PR have tests?

[x] Does the PR follow project code formatting standards?

[x] Does the OS still compile and run?

Is documentation included?

Code Checks:

[x] Does the code achieve its intended purpose?

[] Were all edge cases considered?

[x] Is the code organized and maintainable?

[x]Does the code maintain the same level of performance?

[MiloCrespo]

CODE REVIEW #1

Checking to make sure the functionality delivered matches the original goals?

Yes, when pulled and launched, the time function and ploop work as explained in the comment.
One way to test a long function without doing anything too crazy is by trying time cat and then just waiting for that to run for some time and then letting it run past. I did not hold out for as long as possuble to see what would make this crash but it it something to consider.

Correcting any minor formatting or documentation issues

In file time.c you use ctime() and just put "a buffer". This was a bit confusing to me and making this more clear would be helpful.

Proposing ways to refactor the code to improve readability, efficiency, or correctness

Less of a suggestion but more question on the use of uint8 instead of uint32, my guess would be the extra large into would not be necessary for a few digit number?

Suggested Constants:
#define MICROSECONDS_PER_SECOND 1000000
#define MILLISECONDS_PER_SECOND 1000
#define SECONDS_PER_MINUTE 60

Possible Updates:
// Current (lines 30-31):
short POSIX = 0;
short arg_start_idx = 1;
uint8 seconds = ...; // line 5

// Updated:
int POSIX = 0; // or bool if available
int arg_start_idx = 1;
uint32 seconds = ...;

You should also verify each item in the following checklist:

High Level Checks:

• Is documentation included? This should explain how to build, run, and test the software.
  • This is included in the /docs folder from what I saw.
• PR fully resolves the task/issue
• Does the PR have tests? - Included in test.md
• Does the PR follow project code formatting standards?
• Does the OS still compile and run?

Code Checks:

• Does the code achieve its intended purpose?
• Were all edge cases considered?
• Is the code organized and maintainable?
• Does the code maintain the same level of performance? (no performance regressions)

[malensek]

This is excellent work and probably more than you expected since it required some kernel modifications. I appreciate the approach you took on tackling this and I like where it ended up (using wait to do this is also what Linux does).

Since we don't have floating point math here, it's okay that you're just approximating the decimal places.

Feedback wise:

• I agree with adding some constants to make it clear what you are doing with time conversions
• printtime is always called three times with the same pattern of args repeating, so you should add a helper function that prints all three using printtime at once. That way you only need to update the textual display once if ever needed in the future.
• Triple ctime() at the beginning would not be necessary with tweaks to your RTC code. Right now it is reading the second half of the RTC first, so you're going to get incorrect values without "priming" the clock that way. This part needs to be fixed before we can merge it.

Final score: 4.75
Can gain +0.25 by addressing the issues above.

[malensek]

Forgot to mention: you are returning 1 on successful exit, but success should be = 0.